EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
drivers/i2c: Add a driver for Semtech SX9324
This adds a new driver for the SX9324 proximity detector device.
BUG=b:172397658 BRANCH=zork TEST=Test sx9324 is working as expect.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ifd582482728a2f535ed85f6696b2f5a4529ba421 --- A src/drivers/i2c/sx9324/Kconfig A src/drivers/i2c/sx9324/Makefile.inc A src/drivers/i2c/sx9324/chip.h A src/drivers/i2c/sx9324/registers.h A src/drivers/i2c/sx9324/sx9324.c 5 files changed, 200 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/47640/1
diff --git a/src/drivers/i2c/sx9324/Kconfig b/src/drivers/i2c/sx9324/Kconfig new file mode 100644 index 0000000..77f6967 --- /dev/null +++ b/src/drivers/i2c/sx9324/Kconfig @@ -0,0 +1,6 @@ +config DRIVERS_I2C_SX9324 + bool + default n + depends on HAVE_ACPI_TABLES + help + Board has a Semtech SX9324 proximity sensor. diff --git a/src/drivers/i2c/sx9324/Makefile.inc b/src/drivers/i2c/sx9324/Makefile.inc new file mode 100644 index 0000000..8811c99 --- /dev/null +++ b/src/drivers/i2c/sx9324/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_I2C_SX9324) += sx9324.c diff --git a/src/drivers/i2c/sx9324/chip.h b/src/drivers/i2c/sx9324/chip.h new file mode 100644 index 0000000..f813c0f --- /dev/null +++ b/src/drivers/i2c/sx9324/chip.h @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __DRIVERS_I2C_SX9324_CHIP_H__ +#define __DRIVERS_I2C_SX9324_CHIP_H__ + +#include <acpi/acpi_device.h> +#include <device/i2c_simple.h> + +#define REGISTER(NAME) uint8_t NAME + +struct drivers_i2c_sx9324_config { + /* ACPI _HID */ + const char *hid; + + /* Device Name */ + const char *name; + + /* Device Description */ + const char *desc; + + /* ACPI _UID */ + unsigned int uid; + + /* Bus speed in Hz, default is I2C_SPEED_FAST */ + enum i2c_speed speed; + + /* Use GPIO-based interrupt instead of IO-APIC */ + struct acpi_gpio irq_gpio; + + /* IO-APIC interrupt */ + struct acpi_irq irq; +#include "registers.h" +}; + +#undef REGISTER + +#endif /* __DRIVERS_I2C_SX9324_CHIP_H__ */ diff --git a/src/drivers/i2c/sx9324/registers.h b/src/drivers/i2c/sx9324/registers.h new file mode 100644 index 0000000..8098648 --- /dev/null +++ b/src/drivers/i2c/sx9324/registers.h @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef REGISTER +#error "define REGISTER(NAME) before including this file" +#endif + +REGISTER(reg_gnrl_ctrl0); +REGISTER(reg_gnrl_ctrl1); + +REGISTER(reg_afe_ctrl0); +REGISTER(reg_afe_ctrl1); +REGISTER(reg_afe_ctrl2); +REGISTER(reg_afe_ctrl3); +REGISTER(reg_afe_ctrl4); +REGISTER(reg_afe_ctrl5); +REGISTER(reg_afe_ctrl6); +REGISTER(reg_afe_ctrl7); +REGISTER(reg_afe_ctrl8); +REGISTER(reg_afe_ctrl9); + +REGISTER(reg_prox_ctrl0); +REGISTER(reg_prox_ctrl1); +REGISTER(reg_prox_ctrl2); +REGISTER(reg_prox_ctrl3); +REGISTER(reg_prox_ctrl4); +REGISTER(reg_prox_ctrl5); +REGISTER(reg_prox_ctrl6); +REGISTER(reg_prox_ctrl7); + +REGISTER(reg_adv_ctrl0); +REGISTER(reg_adv_ctrl1); +REGISTER(reg_adv_ctrl2); +REGISTER(reg_adv_ctrl3); +REGISTER(reg_adv_ctrl4); +REGISTER(reg_adv_ctrl5); +REGISTER(reg_adv_ctrl6); +REGISTER(reg_adv_ctrl7); +REGISTER(reg_adv_ctrl8); +REGISTER(reg_adv_ctrl9); +REGISTER(reg_adv_ctrl10); +REGISTER(reg_adv_ctrl11); +REGISTER(reg_adv_ctrl12); +REGISTER(reg_adv_ctrl13); +REGISTER(reg_adv_ctrl14); +REGISTER(reg_adv_ctrl15); +REGISTER(reg_adv_ctrl16); +REGISTER(reg_adv_ctrl17); +REGISTER(reg_adv_ctrl18); +REGISTER(reg_adv_ctrl19); +REGISTER(reg_adv_ctrl20); + + diff --git a/src/drivers/i2c/sx9324/sx9324.c b/src/drivers/i2c/sx9324/sx9324.c new file mode 100644 index 0000000..8e6e83f --- /dev/null +++ b/src/drivers/i2c/sx9324/sx9324.c @@ -0,0 +1,104 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpi_device.h> +#include <acpi/acpigen.h> +#include <console/console.h> +#include <device/i2c_simple.h> +#include <device/device.h> +#include <device/path.h> +#include <string.h> +#include "chip.h" + +#define I2C_SX9324_ACPI_ID "STH9324" +#define I2C_SX9324_ACPI_NAME "Semtech SX9324" + +#define REGISTER(NAME) acpi_dp_add_integer(dsd, \ + I2C_SX9324_ACPI_ID "," #NAME, \ + config->NAME) + +static void i2c_sx9324_fill_ssdt(const struct device *dev) +{ + struct drivers_i2c_sx9324_config *config = dev->chip_info; + const char *scope = acpi_device_scope(dev); + struct acpi_i2c i2c = { + .address = dev->path.i2c.device, + .mode_10bit = dev->path.i2c.mode_10bit, + .speed = I2C_SPEED_FAST, + .resource = scope, + }; + struct acpi_dp *dsd; + + if (!scope || !config) + return; + + if (config->speed) + i2c.speed = config->speed; + + /* Device */ + acpigen_write_scope(scope); + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_name_string("_HID", I2C_SX9324_ACPI_ID); + acpigen_write_name_integer("_UID", config->uid); + acpigen_write_name_string("_DDN", config->desc); + acpigen_write_STA(acpi_device_status(dev)); + + /* Resources */ + acpigen_write_name("_CRS"); + acpigen_write_resourcetemplate_header(); + acpi_device_write_i2c(&i2c); + + if (config->irq_gpio.pin_count) + acpi_device_write_gpio(&config->irq_gpio); + else + acpi_device_write_interrupt(&config->irq); + + acpigen_write_resourcetemplate_footer(); + + /* DSD */ + dsd = acpi_dp_new_table("_DSD"); +#include "registers.h" + acpi_dp_write(dsd); + + acpigen_pop_len(); /* Device */ + acpigen_pop_len(); /* Scope */ + + printk(BIOS_INFO, "%s: %s at %s\n", acpi_device_path(dev), + config->desc ? : dev->chip_ops->name, dev_path(dev)); +} + +#undef REGISTER + +static const char *i2c_sx9324_acpi_name(const struct device *dev) +{ + static char name[5]; + + snprintf(name, sizeof(name), "D%03.3X", dev->path.i2c.device); + return name; +} + +static struct device_operations i2c_sx9324_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .acpi_name = i2c_sx9324_acpi_name, + .acpi_fill_ssdt = i2c_sx9324_fill_ssdt, +}; + +static void i2c_sx9324_enable(struct device *dev) +{ + struct drivers_i2c_sx9324_config *config = dev->chip_info; + + if (!config) { + dev->enabled = 0; + return; + } + + dev->ops = &i2c_sx9324_ops; + + if (config->desc) + dev->name = config->desc; +} + +struct chip_operations drivers_i2c_sx9324_ops = { + CHIP_NAME(I2C_SX9324_ACPI_NAME) + .enable_dev = i2c_sx9324_enable +};
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47640
to look at the new patch set (#2).
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
drivers/i2c: Add a driver for Semtech SX9324
This adds a new driver for the SX9324 proximity detector device.
BUG=b:172397658 BRANCH=zork TEST=Test sx9324 is working as expect.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ifd582482728a2f535ed85f6696b2f5a4529ba421 --- A src/drivers/i2c/sx9324/Kconfig A src/drivers/i2c/sx9324/Makefile.inc A src/drivers/i2c/sx9324/chip.h A src/drivers/i2c/sx9324/registers.h A src/drivers/i2c/sx9324/sx9324.c 5 files changed, 198 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/47640/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 2:
This is just copied from sx9310. And _DSD and other are the same with my previous CB:47639. That is the reason I want to combine it.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 2:
(2 comments)
Patch Set 2:
This is just copied from sx9310. And _DSD and other are the same with my previous CB:47639. That is the reason I want to combine it.
Understood. The reason why I said we should create a separate driver is: 1. Ease of organization in devicetree i.e. instead of writing `register "reg.name" = "<name>" register "reg.value" = "<value>"`, you can simply have `register "name" = "<value>". Easier to read as well in my opinion. 2. I was not sure about the expectations of latest kernel for sx9310. But, it looks like there is work ongoing to make 5.4 driver for sx9310 use ACPI data like 4.4 driver does.
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... File src/drivers/i2c/sx9324/chip.h:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... PS2, Line 12: /* ACPI _HID */ : const char *hid; Not required. This is not used by the driver.
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... PS2, Line 15: /* Device Name */ : const char *name; Unused in the driver.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... File src/drivers/i2c/sx9324/chip.h:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... PS2, Line 12: /* ACPI _HID */ : const char *hid;
Not required. This is not used by the driver.
I see bindings for 3 HIDs in 5.4 (drivers/iio/proximity/sx932x.c): STH9320, STH9321, STH9324
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... File src/drivers/i2c/sx9324/sx9324.c:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... PS2, Line 75: D%03.3X This is pretty generic; from the datasheet, it will be "D028" almost all of the time 😊 What about a more unique prefix, like "SX" ?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... File src/drivers/i2c/sx9324/sx9324.c:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... PS2, Line 75: D%03.3X
This is pretty generic; from the datasheet, it will be "D028" almost all of the time 😊 […]
If we just define "SX93"?
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47640
to look at the new patch set (#3).
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
drivers/i2c: Add a driver for Semtech SX9324
This adds a new driver for the SX9324 proximity detector device.
BUG=b:172397658 BRANCH=zork TEST=Test sx9324 is working as expect.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ifd582482728a2f535ed85f6696b2f5a4529ba421 --- A src/drivers/i2c/sx9324/Kconfig A src/drivers/i2c/sx9324/Makefile.inc A src/drivers/i2c/sx9324/chip.h A src/drivers/i2c/sx9324/registers.h A src/drivers/i2c/sx9324/sx9324.c 5 files changed, 190 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/47640/3
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... File src/drivers/i2c/sx9324/chip.h:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... PS2, Line 12: /* ACPI _HID */ : const char *hid;
I see bindings for 3 HIDs in 5.4 (drivers/iio/proximity/sx932x. […]
We will check the kernel driver in the same time. But now just specific to sx9324.
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... PS2, Line 15: /* Device Name */ : const char *name;
Unused in the driver.
Done
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... File src/drivers/i2c/sx9324/sx9324.c:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... PS2, Line 75: D%03.3X
If we just define "SX93"?
Done
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... File src/drivers/i2c/sx9324/chip.h:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... PS2, Line 12: /* ACPI _HID */ : const char *hid;
We will check the kernel driver in the same time. But now just specific to sx9324.
IMO, if we want to support all 3. We should use kconfig to define the prefix I2C_SX9324_ACPI_ID. We can do it later if we have another two sensor use in other project.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... File src/drivers/i2c/sx9324/sx9324.c:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... PS2, Line 75: D%03.3X
Done
Could be more than one... there is a potential use-case for a SAR for WiFi and a SAR for LTE, so if they're on the same I2C bus, we'd have a name conflict then.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47640/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47640/3//COMMIT_MSG@10 PS3, Line 10: Plesae add the name and revision of the datasheet.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47640
to look at the new patch set (#4).
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
drivers/i2c: Add a driver for Semtech SX9324
This adds a new driver for the SX9324 proximity detector device. Follow SX9324 datasheet Rev3.
BUG=b:172397658 BRANCH=zork TEST=Test sx9324 is working as expect.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ifd582482728a2f535ed85f6696b2f5a4529ba421 --- A src/drivers/i2c/sx9324/Kconfig A src/drivers/i2c/sx9324/Makefile.inc A src/drivers/i2c/sx9324/chip.h A src/drivers/i2c/sx9324/registers.h A src/drivers/i2c/sx9324/sx9324.c 5 files changed, 190 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/47640/4
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47640/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47640/3//COMMIT_MSG@10 PS3, Line 10:
Plesae add the name and revision of the datasheet.
Done
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... File src/drivers/i2c/sx9324/sx9324.c:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/sx93... PS2, Line 75: D%03.3X
Could be more than one... […]
Make sense. change to SX%2x.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47640
to look at the new patch set (#5).
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
drivers/i2c: Add a driver for Semtech SX9324
This adds a new driver for the SX9324 proximity detector device. Follow SX9324 datasheet Rev3.
BUG=b:172397658 BRANCH=zork TEST=Test sx9324 is working as expect.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ifd582482728a2f535ed85f6696b2f5a4529ba421 --- A src/drivers/i2c/sx9324/Kconfig A src/drivers/i2c/sx9324/Makefile.inc A src/drivers/i2c/sx9324/chip.h A src/drivers/i2c/sx9324/registers.h A src/drivers/i2c/sx9324/sx9324.c 5 files changed, 192 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/47640/5
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... File src/drivers/i2c/sx9324/chip.h:
https://review.coreboot.org/c/coreboot/+/47640/2/src/drivers/i2c/sx9324/chip... PS2, Line 15: /* Device Name */ : const char *name;
Done
@Tim, uid can reflect each HIDs then driver will try the correspond property. I've tried print some log in kernel. In the feature we should care uid for this driver as well:)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/47640/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47640/5//COMMIT_MSG@14 PS5, Line 14: expect nit: expect*ed*
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47640
to look at the new patch set (#6).
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
drivers/i2c: Add a driver for Semtech SX9324
This adds a new driver for the SX9324 proximity detector device. Follow SX9324 datasheet Rev3.
BUG=b:172397658 BRANCH=zork TEST=Test sx9324 is working as expected.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ifd582482728a2f535ed85f6696b2f5a4529ba421 --- A src/drivers/i2c/sx9324/Kconfig A src/drivers/i2c/sx9324/Makefile.inc A src/drivers/i2c/sx9324/chip.h A src/drivers/i2c/sx9324/registers.h A src/drivers/i2c/sx9324/sx9324.c 5 files changed, 192 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/47640/6
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47640/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47640/5//COMMIT_MSG@14 PS5, Line 14: expect
nit: expect*ed*
Done
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6:
@Tim, would you take a look again? Thanks.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6:
Patch Set 6:
@Tim, would you take a look again? Thanks.
One of my main concerns with this patch is that it is currently not build-tested. If there's a board in the tree that will use this code, mind pushing a follow-up to this change to select this driver in its Kconfig, please?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
@Tim, would you take a look again? Thanks.
One of my main concerns with this patch is that it is currently not build-tested. If there's a board in the tree that will use this code, mind pushing a follow-up to this change to select this driver in its Kconfig, please?
Yes, will commit follow CL after this CL get merged. Still fine tune some registers for SAR. We verified this on Zork board with kernel v5.4 :)
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6: Code-Review+2
Sorry for the delay Eric, let's get this hooked up to a board soon?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6:
Patch Set 6: Code-Review+2
Sorry for the delay Eric, let's get this hooked up to a board soon?
sure 😉
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6:
Plan to submit the draft version next week, but due to the RF antenna schedule. The final register value will delay for weeks.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
Patch Set 6:
Patch Set 6:
Plan to submit the draft version next week, but due to the RF antenna schedule. The final register value will delay for weeks.
SG!
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47640 )
Change subject: drivers/i2c: Add a driver for Semtech SX9324 ......................................................................
drivers/i2c: Add a driver for Semtech SX9324
This adds a new driver for the SX9324 proximity detector device. Follow SX9324 datasheet Rev3.
BUG=b:172397658 BRANCH=zork TEST=Test sx9324 is working as expected.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ifd582482728a2f535ed85f6696b2f5a4529ba421 Reviewed-on: https://review.coreboot.org/c/coreboot/+/47640 Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- A src/drivers/i2c/sx9324/Kconfig A src/drivers/i2c/sx9324/Makefile.inc A src/drivers/i2c/sx9324/chip.h A src/drivers/i2c/sx9324/registers.h A src/drivers/i2c/sx9324/sx9324.c 5 files changed, 192 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/drivers/i2c/sx9324/Kconfig b/src/drivers/i2c/sx9324/Kconfig new file mode 100644 index 0000000..77f6967 --- /dev/null +++ b/src/drivers/i2c/sx9324/Kconfig @@ -0,0 +1,6 @@ +config DRIVERS_I2C_SX9324 + bool + default n + depends on HAVE_ACPI_TABLES + help + Board has a Semtech SX9324 proximity sensor. diff --git a/src/drivers/i2c/sx9324/Makefile.inc b/src/drivers/i2c/sx9324/Makefile.inc new file mode 100644 index 0000000..8811c99 --- /dev/null +++ b/src/drivers/i2c/sx9324/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_I2C_SX9324) += sx9324.c diff --git a/src/drivers/i2c/sx9324/chip.h b/src/drivers/i2c/sx9324/chip.h new file mode 100644 index 0000000..9c9dd3c --- /dev/null +++ b/src/drivers/i2c/sx9324/chip.h @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __DRIVERS_I2C_SX9324_CHIP_H__ +#define __DRIVERS_I2C_SX9324_CHIP_H__ + +#include <acpi/acpi_device.h> +#include <device/i2c_simple.h> + +#define REGISTER(NAME) uint8_t NAME + +struct drivers_i2c_sx9324_config { + /* Device Description */ + const char *desc; + + /* ACPI _UID */ + unsigned int uid; + + /* Bus speed in Hz, default is I2C_SPEED_FAST */ + enum i2c_speed speed; + + /* Use GPIO-based interrupt instead of IO-APIC */ + struct acpi_gpio irq_gpio; + + /* IO-APIC interrupt */ + struct acpi_irq irq; +#include "registers.h" +}; + +#undef REGISTER + +#endif /* __DRIVERS_I2C_SX9324_CHIP_H__ */ diff --git a/src/drivers/i2c/sx9324/registers.h b/src/drivers/i2c/sx9324/registers.h new file mode 100644 index 0000000..160e5e7 --- /dev/null +++ b/src/drivers/i2c/sx9324/registers.h @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef REGISTER +#error "define REGISTER(NAME) before including this file" +#endif + +REGISTER(reg_gnrl_ctrl0); +REGISTER(reg_gnrl_ctrl1); + +REGISTER(reg_afe_ctrl0); +REGISTER(reg_afe_ctrl1); +REGISTER(reg_afe_ctrl2); +REGISTER(reg_afe_ctrl3); +REGISTER(reg_afe_ctrl4); +REGISTER(reg_afe_ctrl5); +REGISTER(reg_afe_ctrl6); +REGISTER(reg_afe_ctrl7); +REGISTER(reg_afe_ctrl8); +REGISTER(reg_afe_ctrl9); + +REGISTER(reg_prox_ctrl0); +REGISTER(reg_prox_ctrl1); +REGISTER(reg_prox_ctrl2); +REGISTER(reg_prox_ctrl3); +REGISTER(reg_prox_ctrl4); +REGISTER(reg_prox_ctrl5); +REGISTER(reg_prox_ctrl6); +REGISTER(reg_prox_ctrl7); + +REGISTER(reg_adv_ctrl0); +REGISTER(reg_adv_ctrl1); +REGISTER(reg_adv_ctrl2); +REGISTER(reg_adv_ctrl3); +REGISTER(reg_adv_ctrl4); +REGISTER(reg_adv_ctrl5); +REGISTER(reg_adv_ctrl6); +REGISTER(reg_adv_ctrl7); +REGISTER(reg_adv_ctrl8); +REGISTER(reg_adv_ctrl9); +REGISTER(reg_adv_ctrl10); +REGISTER(reg_adv_ctrl11); +REGISTER(reg_adv_ctrl12); +REGISTER(reg_adv_ctrl13); +REGISTER(reg_adv_ctrl14); +REGISTER(reg_adv_ctrl15); +REGISTER(reg_adv_ctrl16); +REGISTER(reg_adv_ctrl17); +REGISTER(reg_adv_ctrl18); +REGISTER(reg_adv_ctrl19); +REGISTER(reg_adv_ctrl20); diff --git a/src/drivers/i2c/sx9324/sx9324.c b/src/drivers/i2c/sx9324/sx9324.c new file mode 100644 index 0000000..2ca9f36 --- /dev/null +++ b/src/drivers/i2c/sx9324/sx9324.c @@ -0,0 +1,104 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpi_device.h> +#include <acpi/acpigen.h> +#include <console/console.h> +#include <device/i2c_simple.h> +#include <device/device.h> +#include <device/path.h> +#include <string.h> +#include "chip.h" + +#define I2C_SX9324_ACPI_ID "STH9324" +#define I2C_SX9324_CHIP_NAME "Semtech SX9324" + +#define REGISTER(NAME) acpi_dp_add_integer(dsd, \ + I2C_SX9324_ACPI_ID "," #NAME, \ + config->NAME) + +static void i2c_sx9324_fill_ssdt(const struct device *dev) +{ + struct drivers_i2c_sx9324_config *config = dev->chip_info; + const char *scope = acpi_device_scope(dev); + struct acpi_i2c i2c = { + .address = dev->path.i2c.device, + .mode_10bit = dev->path.i2c.mode_10bit, + .speed = I2C_SPEED_FAST, + .resource = scope, + }; + struct acpi_dp *dsd; + + if (!scope || !config) + return; + + if (config->speed) + i2c.speed = config->speed; + + /* Device */ + acpigen_write_scope(scope); + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_name_string("_HID", I2C_SX9324_ACPI_ID); + acpigen_write_name_integer("_UID", config->uid); + acpigen_write_name_string("_DDN", config->desc); + acpigen_write_STA(acpi_device_status(dev)); + + /* Resources */ + acpigen_write_name("_CRS"); + acpigen_write_resourcetemplate_header(); + acpi_device_write_i2c(&i2c); + + if (config->irq_gpio.pin_count) + acpi_device_write_gpio(&config->irq_gpio); + else + acpi_device_write_interrupt(&config->irq); + + acpigen_write_resourcetemplate_footer(); + + /* DSD */ + dsd = acpi_dp_new_table("_DSD"); +#include "registers.h" + acpi_dp_write(dsd); + + acpigen_pop_len(); /* Device */ + acpigen_pop_len(); /* Scope */ + + printk(BIOS_INFO, "%s: %s at %s\n", acpi_device_path(dev), + config->desc ? : dev->chip_ops->name, dev_path(dev)); +} + +#undef REGISTER + +static const char *i2c_sx9324_acpi_name(const struct device *dev) +{ + static char name[5]; + + snprintf(name, sizeof(name), "SX%02.2X", dev->path.i2c.device); + return name; +} + +static struct device_operations i2c_sx9324_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .acpi_name = i2c_sx9324_acpi_name, + .acpi_fill_ssdt = i2c_sx9324_fill_ssdt, +}; + +static void i2c_sx9324_enable(struct device *dev) +{ + struct drivers_i2c_sx9324_config *config = dev->chip_info; + + if (!config) { + dev->enabled = 0; + return; + } + + dev->ops = &i2c_sx9324_ops; + + if (config->desc) + dev->name = config->desc; +} + +struct chip_operations drivers_i2c_sx9324_ops = { + CHIP_NAME(I2C_SX9324_CHIP_NAME) + .enable_dev = i2c_sx9324_enable +};
-
Angel Pons (Code Review) -
EricR Lai (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)