Date: Mon, 24 Aug 2015 14:31:14 +0200
From: Johan Hovold <johan@kernel.org>
To: =?iso-8859-1?Q?Bj=F8rn?= Mork <bjorn@mork.no>
Cc: Johan Hovold <johan@kernel.org>, "Liu.Zhao" <lzsos369@163.com>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        gregkh@linuxfoundation.org, konstantin@linuxfoundation.org,
        398817832@qq.com, 278883616@qq.com, yang.haojun3@zte.com.cn
Subject: Re: [PATCH v3 1/1] USB:option:add ZTE PIDs
Message-ID: <20150824123114.GE14209@localhost>
References: <1439999477-3447-1-git-send-email-lzsos369@163.com>
 <20150824072636.GD14209@localhost>
 <87a8th6s56.fsf@nemi.mork.no>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <87a8th6s56.fsf@nemi.mork.no>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1977
Lines: 60

On Mon, Aug 24, 2015 at 09:51:33AM +0200, Bj?rn Mork wrote:
> Johan Hovold <johan@kernel.org> writes:
> > On Wed, Aug 19, 2015 at 08:51:17AM -0700, Liu.Zhao wrote:
> >> 
> >>  #define BENQ_VENDOR_ID				0x04a5
> >>  #define BENQ_PRODUCT_H10			0x4068
> >> @@ -544,6 +548,14 @@ static const struct option_blacklist_info zte_mc2716_z_blacklist = {
> >>  	.sendsetup = BIT(1) | BIT(2) | BIT(3),
> >>  };
> >>  
> >> +static const struct option_blacklist_info zte_me3620andzm8620_xl_blacklist = {
> >> +	.reserved = BIT(3) | BIT(4) | BIT(5),
> >> +};
> >
> > Use two structs for this: zte_me3620_blacklist and zm8620_xl_blacklist
> > even if they reserve the same ports.
> 
> Why?

To avoid including every device family in the symbol name (and we
already have duplicate blacklist definitions).

> Wouldn't it be better to merge all identical lists and give them
> structured names describing their contents instead?

It certainly would.

> E.g.
> 
>  static const struct option_blacklist_info bi_s0001_r = {
>         .sendsetup = BIT(0) | BIT(1),
>  };
> 
>  static const struct option_blacklist_info bi_s0001_r04 = {
>         .sendsetup = BIT(0) | BIT(1),
>         .reserved = BIT(4),
>  };
> 
>  static const struct option_blacklist_info bi_s_r030405 =  {
> 	.reserved = BIT(3) | BIT(4) | BIT(5),
>  };
> 
> 
> etc.  Or some other naming scheme.

Perhaps bi_s<setup_mask>_r<reserved_mask> (e.g. bi_s3_r0, bi_s3_r10, and
bi_s0_r38 for the above) would be too compact?

> I don't see the point of having lots of identical structs just to be
> able to name them after some rarely meaningful marketing name.  Many
> vendors recycle their pids, making this completely futile.

I agree. Let's just decide on a naming scheme first.

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/