Re: Re: [PATCH v3 4/4] media: ox05b1s: Add support for Omnivision OS08A20 raw sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Krzysztof,

thanks again for feedback.

On 24.01.2025 10:09, Krzysztof Kozlowski wrote:
On Fri, Jan 24, 2025 at 02:12:43AM +0200, Mirela Rabulea wrote:
@@ -758,6 +914,9 @@ static int ox05b1s_read_chip_id(struct ox05b1s *sensor)
       }

       switch (chip_id) {
+     case 0x530841:
+             camera_name = "os08a20";
+             break;
Ah, so here I see missing second device support.

It is still confusing to see that you use here some sort of
autodetection, but in the same time not.

The two sensors that I included in this driver have some similarities, but also differences, for which I used the platform data. I made separate patches for the two sensors, such that it is visible how much is common/different.  The chip_id reading is for validating that the sensor that is actually attached matches the device tree. It happens to me sometimes, that I switch the sensors, but forget to switch the dtb, and it helps to see which sensor is actually attached. Also, it helps a lot when the evaluation board is in some remote lab and I am unsure what sensor is attached to it.

I saw most sensor drivers have some kind of identification of the sensor module by the means of reading the chip_id register. Some examples with multiple compatibles supported and chip_id validation: imx296, ov9650, ov772x.

Please let me know what you suggest further.

For your other comments, I did prepare more structures to become const, hope I did not miss any, it will be in the next version.

Thanks,

Mirela



       case 0x580542:
               camera_name = "ox05b1s";
               break;
@@ -900,6 +1059,24 @@ static void ox05b1s_remove(struct i2c_client *client)
  static DEFINE_RUNTIME_DEV_PM_OPS(ox05b1s_pm_ops, ox05b1s_runtime_suspend,
                                ox05b1s_runtime_resume, NULL);

+static const struct ox05b1s_plat_data os08a20_data = {
+     .name                   = "os08a20",
+     .chip_id                = 0x530841,
+     .native_width           = 3872, /* 16 dummy + 3840 active pixels + 16 dummy */
+     .native_height          = 2192, /* 16 dummy + 2160 active lines + 16 dummy */
+     .active_top             = 16,
+     .active_left            = 16,
+     .active_width           = 3840,
+     .active_height          = 2160,
+     .supported_modes        = os08a20_supported_modes,
+     .default_mode_index     = 0,
+     .supported_codes        = os08a20_supported_codes,
+     .hdr_modes              = os08a20_hdr_modes,
+     .hdr_modes_count        = ARRAY_SIZE(os08a20_hdr_modes),
+     .set_hdr_mode           = os08a20_set_hdr_mode,
+
+};
+
  static const struct ox05b1s_plat_data ox05b1s_data = {
       .name                   = "ox05b1s",
       .chip_id                = 0x580542,
@@ -912,9 +1089,16 @@ static const struct ox05b1s_plat_data ox05b1s_data = {
       .supported_modes        = ox05b1s_supported_modes,
       .default_mode_index     = 0,
       .supported_codes        = ox05b1s_supported_codes,
+     .hdr_modes              = NULL,
+     .hdr_modes_count        = 0,
+     .set_hdr_mode           = NULL,
  };

  static const struct of_device_id ox05b1s_of_match[] = {
+     {
+             .compatible = "ovti,os08a20",
+             .data = &os08a20_data,
And here static configuration of model, not autodetection.


Best regards,
Krzysztof





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux