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

From: Johan Hovold
Date: Mon Aug 24 2015 - 08:31:30 EST


On Mon, Aug 24, 2015 at 09:51:33AM +0200, Bjørn Mork wrote:
> Johan Hovold <johan@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/