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.