Date: Mon, 17 Aug 2015 12:49:41 +0200
From: Johan Hovold <johan@kernel.org>
To: "Liu.Zhao" <lzsos369@163.com>
Cc: johan@kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        gregkh@linuxfoundation.org, konstantin@linuxfoundation.org,
        398817832@qq.com, 278883616@qq.com
Subject: Re: [PATCH 1/1] add-ZTE-pid
Message-ID: <20150817104941.GA7053@localhost>
References: <1438573308-14750-1-git-send-email-lzsos369@163.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1438573308-14750-1-git-send-email-lzsos369@163.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3485
Lines: 98

On Sun, Aug 02, 2015 at 08:41:48PM -0700, Liu.Zhao wrote:

Make sure to always include a commit message.

Also change you Subject (patch summary) to something more descriptive
using the following format:

	USB: option: add ZTE PIDs

> Signed-off-by: Liu.Zhao <lzsos369@163.com>
> ---
>  drivers/usb/serial/option.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index f0c0c53..6996308 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -285,6 +285,10 @@ static void option_instat_callback(struct urb *urb);
>  #define ZTE_PRODUCT_MC2718			0xffe8
>  #define ZTE_PRODUCT_AD3812			0xffeb
>  #define ZTE_PRODUCT_MC2716			0xffed
> +#define ZTE_PRODUCT_ZM8620_X			0x0396
> +#define ZTE_PRODUCT_ME3620_X			0x1432
> +#define ZTE_PRODUCT_ME3620_L			0x1433
> +#define ZTE_PRODUCT_ME3620_MBIM		0x0426

I already asked you to try to keep these sorted according to PID (e.g.
add them sorted before ZTE_PRODUCT_AC2726).

>  
>  #define BENQ_VENDOR_ID				0x04a5
>  #define BENQ_PRODUCT_H10			0x4068
> @@ -544,6 +548,23 @@ static const struct option_blacklist_info zte_mc2716_z_blacklist = {
>  	.sendsetup = BIT(1) | BIT(2) | BIT(3),
>  };
>  
> +static const struct option_blacklist_info zte_zm8620_x_blacklist = {
> +	.reserved = BIT(3) | BIT(4) | BIT(5),
> +};
> +
> +static const struct option_blacklist_info zte_me3620_x_blacklist = {
> +	.reserved = BIT(3) | BIT(4) | BIT(5),
> +};
> +
> +static const struct option_blacklist_info zte_me3620_l_blacklist = {
> +	.reserved = BIT(3) | BIT(4) | BIT(5),
> +};
> +
> +static const struct option_blacklist_info zte_me3620_mbim_blacklist = {
> +	.reserved = BIT(2) | BIT(3) | BIT(4),
> +};

I also asked you to consider reusing the blacklist structs for related
devices (e.g. zte_me3620_xl).

Please keep these new structs sorted by symbol name as well.

> +
> +

Remove the stray newline.

>  static const struct option_blacklist_info huawei_cdc12_blacklist = {
>  	.reserved = BIT(1) | BIT(2),
>  };
> @@ -1592,6 +1613,14 @@ static const struct usb_device_id option_ids[] = {
>  	{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x01) },
>  	{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x05) },
>  	{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x86, 0x10) },
> +	{ USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ZM8620_X),
> +		.driver_info = (kernel_ulong_t)&zte_zm8620_x_blacklist },
> +	{ USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_X),
> +		.driver_info = (kernel_ulong_t)&zte_me3620_x_blacklist },
> +	{ USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_L),
> +		.driver_info = (kernel_ulong_t)&zte_me3620_l_blacklist },
> +	{ USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_MBIM),
> +		.driver_info = (kernel_ulong_t)&zte_me3620_mbim_blacklist },

And try to keep these entries sorted on symbol names as well (e.g. add
them sorted after ZTE_PRODUCT_MC2716).

>  	{ USB_DEVICE(BENQ_VENDOR_ID, BENQ_PRODUCT_H10) },
>  	{ USB_DEVICE(DLINK_VENDOR_ID, DLINK_PRODUCT_DWM_652) },

Otherwise the patch is now on the right format.

Could you fix up the above and resend?

Please include a "[PATCH v3]"-prefix in you subject when resending.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/