Hi Laurent,
Thanks for your suggestion.
>> +static const struct mt9p031_model_info mt9p031_models[] = {
>> + {.code = MEDIA_BUS_FMT_SGRBG12_1X12}, /* mt9p006 */
>
>There should be spaces after { and before }
okay.
>> + {.code = MEDIA_BUS_FMT_SGRBG12_1X12}, /* mt9p031 */
>
>You can use the same entry for both the MT9P006 and MT9P031 as they
>don't need to be deferentiated.
sure, will do
>> + {.code = MEDIA_BUS_FMT_Y12_1X12}, /* mt9p031m */
>> +};
>> +
[...]
>> static const struct i2c_device_id mt9p031_id[] = {
>> - { "mt9p006", MEDIA_BUS_FMT_SGRBG12_1X12 },
>> - { "mt9p031", MEDIA_BUS_FMT_SGRBG12_1X12 },
>> - { "mt9p031m", MEDIA_BUS_FMT_Y12_1X12 },
>> + { "mt9p006", 0 },
>> + { "mt9p031", 1 },
>> + { "mt9p031m", 2 },
>> { /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(i2c, mt9p031_id);
>
>I think we can drop mt9p031_id. I'll send a patch series to do so.
okay
>> static const struct of_device_id mt9p031_of_match[] = {
>> - { .compatible = "aptina,mt9p006", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
>> - { .compatible = "aptina,mt9p031", .data = (void *)MEDIA_BUS_FMT_SGRBG12_1X12 },
>> - { .compatible = "aptina,mt9p031m", .data = (void *)MEDIA_BUS_FMT_Y12_1X12 },
>> + { .compatible = "aptina,mt9p006", .data = (void *)0 },
>> + { .compatible = "aptina,mt9p031", .data = (void *)1 },
>> + { .compatible = "aptina,mt9p031m", .data = (void *)2 },
>
>Let's avoid magic values. You can write
>
> { .compatible = "aptina,mt9p006", .data = &mt9p031_models[0] },
> { .compatible = "aptina,mt9p031", .data = &mt9p031_models[0] },
> { .compatible = "aptina,mt9p031m", .data = &mt9p031_models[1] },
>
>but it may be even more readable to introduce a
>
>enum mt9p031_model {
> MT9P031_MODEL_BAYER,
> MT9P031_MODEL_MONO,
>};
makes sense.
>static const struct mt9p031_model_info mt9p031_models[] = {
> [MT9P031_MODEL_BAYER] = {
> .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> },
> [MT9P031_MODEL_MONO] = {
> .code = MEDIA_BUS_FMT_Y12_1X12,
> },
>};
>
>static const struct of_device_id mt9p031_of_match[] = {
> {
> .compatible = "aptina,mt9p006",
> .data = &mt9p031_models[MT9P031_MODEL_BAYER],
> }, {
> .compatible = "aptina,mt9p031",
> .data = &mt9p031_models[MT9P031_MODEL_BAYER],
> }, {
> .compatible = "aptina,mt9p031m",
> .data = &mt9p031_models[MEDIA_BUS_FMT_Y12_1X12],
> },
> { /* sentinel */ }
>};
I will update as per your suggestions and also drop mt9p031_id.
Best Regards,
Tarang
