On Wed, 2015-06-24 at 11:55 +0900, Taku Izumi wrote: > This patch adds the basic code of FUJITSU Extended Socket > Network Device driver. trivial notes: > diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c [] > +static int fjes_acpi_add(struct acpi_device *device) > +{ [] > + result = > + utf16s_to_utf8s((wchar_t *)str->string.pointer, > + str->string.length, UTF16_LITTLE_ENDIAN, > + str_buf, sizeof(str_buf) - 1); I dislike this style as it's not normally used in the kernel. Most would use another tab indent on the utf16s_to_utf8s line if the line didn't fit, but this would is more common: result = utf16s_to_utf8s((wchar_t *)str->string.pointer, str->string.length, UTF16_LITTLE_ENDIAN, str_buf, sizeof(str_buf) - 1); > > + /* create platform_device */ > + plat_dev = > + platform_device_register_simple(DRV_NAME, 0, fjes_resource, > + ARRAY_SIZE(fjes_resource)); plat_dev = platform_device_register_simple(DRV_NAME, 0, fjes_resource, ARRAY_SIZE(fjes_resource)); etc... > +static acpi_status > +fjes_get_acpi_resource(struct acpi_resource *acpi_res, void *data) > +{ > + struct resource *res = data; > + struct acpi_resource_address32 *addr; > + struct acpi_resource_irq *irq; > + > + switch (acpi_res->type) { > + case ACPI_RESOURCE_TYPE_ADDRESS32: > + addr = &acpi_res->data.address32; > + res[0].start = addr->address.minimum; > + res[0].end = addr->address.minimum + > + addr->address.address_length - 1; > + break; > + > + case ACPI_RESOURCE_TYPE_IRQ: > + irq = &acpi_res->data.irq; > + if (irq->interrupt_count != 1) > + return AE_ERROR; > + res[1].start = irq->interrupts[0]; > + res[1].end = irq->interrupts[0]; > + break; > + > + default: > + break; > + } > + > + return AE_OK; > +} Is this function used in this compilation? If not, it's odd to add code that isn't used. Is it better to make the 2nd argument a struct resource * rather than a void * ? -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html