Thanks for your help. > -----Original Message----- > From: platform-driver-x86-owner@xxxxxxxxxxxxxxx [mailto:platform-driver-x86-owner@xxxxxxxxxxxxxxx] On Behalf Of Joe > Perches > Sent: Wednesday, June 24, 2015 3:51 PM > To: Izumi, Taku/泉 拓 > Cc: netdev@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx; dvhart@xxxxxxxxxxxxx; > rkhan@xxxxxxxxxx; alexander.h.duyck@xxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; sergei.shtylyov@xxxxxxxxxxxxxxxxxx; > stephen@xxxxxxxxxxxxxxxxxx; yasu.isimatu@xxxxxxxxx > Subject: Re: [PATCH v2 01/22] fjes: Introduce FUJITSU Extended Socket Network Device driver > > 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: To tell the truth, I dislike this style too.... OK, I'll fix. I reformatted to fix " CHECK: Alignment should match open parenthesis" style issue. I thought the following style is easy to exceed 80 columns, I opted the above style. > 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 * ? This function is used as a callback function of "acpi_walk_resources". Its signature is acpi_status(*acpi_walk_resource_callback) (struct acpi_resource * resource, void *context); Sincerely, Taku Izumi > > -- > 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 -- 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
