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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 25 Jun 2015 00:32:40 +0000
From:	"Izumi, Taku" <izumi.taku@...fujitsu.com>
To:	Joe Perches <joe@...ches.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	"dvhart@...radead.org" <dvhart@...radead.org>,
	"rkhan@...hat.com" <rkhan@...hat.com>,
	"alexander.h.duyck@...hat.com" <alexander.h.duyck@...hat.com>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"sergei.shtylyov@...entembedded.com" 
	<sergei.shtylyov@...entembedded.com>,
	"stephen@...workplumber.org" <stephen@...workplumber.org>,
	"yasu.isimatu@...il.com" <yasu.isimatu@...il.com>
Subject: RE: [PATCH v2 01/22] fjes: Introduce FUJITSU Extended Socket
 Network Device driver


 Thanks for your help.

> -----Original Message-----
> From: platform-driver-x86-owner@...r.kernel.org [mailto:platform-driver-x86-owner@...r.kernel.org] On Behalf Of Joe
> Perches
> Sent: Wednesday, June 24, 2015 3:51 PM
> To: Izumi, Taku/泉 拓
> Cc: netdev@...r.kernel.org; davem@...emloft.net; platform-driver-x86@...r.kernel.org; dvhart@...radead.org;
> rkhan@...hat.com; alexander.h.duyck@...hat.com; linux-acpi@...r.kernel.org; sergei.shtylyov@...entembedded.com;
> stephen@...workplumber.org; yasu.isimatu@...il.com
> 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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html