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