Dear Guennadi Thank you for checking patch > Can you explain a bit why this patch is needed? Apart from a slight > stylistic improvement and a saving of 4 bytes of platform data per camera > instance? Is it going to be needed for some future changes? This patch is not so important/necessary at once. -> for saving of 4 bytes. > if (!is_power_of_2(priv->info->flags & (OV772X_FLAG_8BIT | OV772X_FLAG_10BIT))) > return 0; > > make sense here? Or even just modify your tests above to (snip) > Adding a "default:" case just above the "case OV772X_FLAG_10BIT:" line > would seem like a good idea to me too. I understand. > > +#define OV772X_FLAG_8BIT (1 << 2) /* 8bit buswidth */ > > +#define OV772X_FLAG_10BIT (1 << 3) /* 10bit buswidth */ May I suggest here ? What do you think if it have only 10BIT flag, and check/operation like this ? if (priv->info->flags & OV772X_FLAG_10BIT) { flags |= SOCAM_DATAWIDTH_10; else flags |= SOCAM_DATAWIDTH_8; This case, below check became not needed, Does this operation make sense for you ? > > /* > > - * ov772x only use 8 or 10 bit bus width > > - */ > > - if (SOCAM_DATAWIDTH_10 != priv->info->buswidth && > > - SOCAM_DATAWIDTH_8 != priv->info->buswidth) { > > - dev_err(&client->dev, "bus width error\n"); > > - return -ENODEV; > > - } Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html