RE: [PATCH v2 01/22] fjes: Introduce FUJITSU Extended Socket Network Device driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 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



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux