Re: [PATCH v3 1/1] USB:option:add ZTE PIDs

[Johan Hovold <johan@kernel.org>]

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