From: Luigi311 <[email protected]>
Resend due to email message limits being exceeded.
v2 changes:
- Add use macros patch
- Add support for powerdown gpio patch
- Add support for reset gpio patch
- Dropped Add support for long exposure modes patch
- Implemented feedback from Jacopo Mondi
- media: i2c: imx258: Add regulator control
- media: i2c: imx258: Add support for 24MHz clock
- media: i2c: imx258: Add support for running on 2 CSI data lanes
- media: i2c: imx258: Add get_selection for pixel array information
- media: i2c: imx258: Issue reset before starting streaming
- media: i2c: imx258: Set pixel_rate range to the same as the value
- dt-bindings: media: imx258: Add alternate compatible strings
- media: i2c: imx258: Change register settings for variants of the sensor
- media: i2c: imx258: Make HFLIP and VFLIP controls writable
This adds a few more patches and drops one. The long exposure mode patch was
dropped due to the bug that Jacopo found. The powerdown and reset gpio patches
were added as that fixes support for the Pinephone Pro, without them the sensor
doesnt initialize correctly.
Tested on a Pinephone Pro by forcing 24 mhz clock and was able to access all 3
resolutions. The two lower resolutions had some artifacts but that is expected
as more changes are required to fix them for the Pinephone Pro specifically,
kept all registers the same as Dave's original patch since that works on
dedicated imx258 hardware and the artifacts are PPP specific so it shouldnt be
a regression.
v1
This is a set of patches for imx258 that allow it to work with alternate clock
frequencies, over either 2 or 4 lanes, and generally adding flexibility to the
driver.
Tested with an IMX258 module from Soho Enterprises that has a 24MHz oscillator.
Both 2 and 4 lane configurations work with correct link frequencies and pixel
rates.
Jacopo has tested on a PinePhone Pro which has an ~19.2MHz clock fed from the SoC,
He confirms that the two lower resolution modes work, but not the full res mode.
Comparing to the BSP it looks like they have some weird clock configuration in
the 4208x3120 mode (nominally 1224Mb/s/lane instead of 1267).
As it has never previously worked directly with the mainline driver this isn't a
regression but may indicate that there is a need for support of additional link
frequencies in the future.
The last patch that makes HFLIP and VFLIP configurable may be contentious as I've
retained the default configuration of inverted from the original driver. I know
this was discussed recently, but I can't recall the final outcome.
I am relying on someone from Intel testing this out, as correcting the cropping
and supporting flips has changed the Bayer order. Seeing as this is all above
board in V4L2 terms I really hope that the layers above it behave themselves.
Cheers
Dave
Dave Stevenson (20):
media: i2c: imx258: Remove unused defines
media: i2c: imx258: Make image geometry meet sensor requirements
media: i2c: imx258: Disable digital cropping on binned modes
media: i2c: imx258: Remove redundant I2C writes.
media: i2c: imx258: Add regulator control
media: i2c: imx258: Make V4L2_CID_VBLANK configurable.
media: i2c: imx258: Split out common registers from the mode based
ones
media: i2c: imx258: Add support for 24MHz clock
media: i2c: imx258: Add support for running on 2 CSI data lanes
media: i2c: imx258: Follow normal V4L2 behaviours for clipping
exposure
media: i2c: imx258: Add get_selection for pixel array information
media: i2c: imx258: Allow configuration of clock lane behaviour
media: i2c: imx258: Correct max FRM_LENGTH_LINES value
media: i2c: imx258: Issue reset before starting streaming
media: i2c: imx258: Set pixel_rate range to the same as the value
media: i2c: imx258: Support faster pixel rate on binned modes
dt-bindings: media: imx258: Rename to include vendor prefix
dt-bindings: media: imx258: Add alternate compatible strings
media: i2c: imx258: Change register settings for variants of the
sensor
media: i2c: imx258: Make HFLIP and VFLIP controls writable
Luigi311 (3):
drivers: media: i2c: imx258: Use macros
drivers: media: i2c: imx258: Add support for powerdown gpio
drivers: media: i2c: imx258: Add support for reset gpio
.../i2c/{imx258.yaml => sony,imx258.yaml} | 12 +-
MAINTAINERS | 2 +-
drivers/media/i2c/imx258.c | 1147 +++++++++++------
3 files changed, 744 insertions(+), 417 deletions(-)
rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (88%)
--
2.42.0
From: Dave Stevenson <[email protected]>
The values and ranges of V4L2_CID_VBLANK are all computed,
so there is no reason for it to be a read only control.
Remove the register values from the mode lists, add the
handler, and remove the read only flag.
Signed-off-by: Dave Stevenson <[email protected]>
---
drivers/media/i2c/imx258.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 495eaada2945..321b504c6a48 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -30,6 +30,8 @@
#define IMX258_VTS_30FPS_VGA 0x034c
#define IMX258_VTS_MAX 0xffff
+#define IMX258_REG_VTS 0x0340
+
/* HBLANK control - read only */
#define IMX258_PPL_DEFAULT 5352
@@ -202,8 +204,6 @@ static const struct imx258_reg mode_4208x3120_regs[] = {
{ 0x0114, 0x03 },
{ 0x0342, 0x14 },
{ 0x0343, 0xE8 },
- { 0x0340, 0x0C },
- { 0x0341, 0x50 },
{ 0x0344, 0x00 },
{ 0x0345, 0x00 },
{ 0x0346, 0x00 },
@@ -319,8 +319,6 @@ static const struct imx258_reg mode_2104_1560_regs[] = {
{ 0x0114, 0x03 },
{ 0x0342, 0x14 },
{ 0x0343, 0xE8 },
- { 0x0340, 0x06 },
- { 0x0341, 0x38 },
{ 0x0344, 0x00 },
{ 0x0345, 0x00 },
{ 0x0346, 0x00 },
@@ -436,8 +434,6 @@ static const struct imx258_reg mode_1048_780_regs[] = {
{ 0x0114, 0x03 },
{ 0x0342, 0x14 },
{ 0x0343, 0xE8 },
- { 0x0340, 0x03 },
- { 0x0341, 0x4C },
{ 0x0344, 0x00 },
{ 0x0345, 0x00 },
{ 0x0346, 0x00 },
@@ -800,6 +796,11 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
BIT(IMX258_HDR_RATIO_MAX));
}
break;
+ case V4L2_CID_VBLANK:
+ ret = imx258_write_reg(imx258, IMX258_REG_VTS,
+ IMX258_REG_VALUE_16BIT,
+ imx258->cur_mode->height + ctrl->val);
+ break;
default:
dev_info(&client->dev,
"ctrl(id:0x%x,val:0x%x) is not handled\n",
@@ -1174,9 +1175,6 @@ static int imx258_init_controls(struct imx258 *imx258)
IMX258_VTS_MAX - imx258->cur_mode->height, 1,
vblank_def);
- if (imx258->vblank)
- imx258->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
-
imx258->hblank = v4l2_ctrl_new_std(
ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_HBLANK,
IMX258_PPL_DEFAULT - imx258->cur_mode->width,
--
2.42.0
From: Dave Stevenson <[email protected]>
Out of all the registers that are defined for each mode, only around
10 differ between the modes.
Split the table into common and mode specific ones.
Signed-off-by: Dave Stevenson <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
drivers/media/i2c/imx258.c | 236 ++++---------------------------------
1 file changed, 21 insertions(+), 215 deletions(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 321b504c6a48..351add1bc5d5 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -151,7 +151,7 @@ static const struct imx258_reg mipi_data_rate_640mbps[] = {
{ 0x0823, 0x00 },
};
-static const struct imx258_reg mode_4208x3120_regs[] = {
+static const struct imx258_reg mode_common_regs[] = {
{ 0x0136, 0x13 },
{ 0x0137, 0x33 },
{ 0x3051, 0x00 },
@@ -216,27 +216,17 @@ static const struct imx258_reg mode_4208x3120_regs[] = {
{ 0x0383, 0x01 },
{ 0x0385, 0x01 },
{ 0x0387, 0x01 },
- { 0x0900, 0x00 },
- { 0x0901, 0x11 },
- { 0x0401, 0x00 },
{ 0x0404, 0x00 },
- { 0x0405, 0x10 },
{ 0x0408, 0x00 },
{ 0x0409, 0x00 },
{ 0x040A, 0x00 },
{ 0x040B, 0x00 },
{ 0x040C, 0x10 },
{ 0x040D, 0x70 },
- { 0x040E, 0x0C },
- { 0x040F, 0x30 },
{ 0x3038, 0x00 },
{ 0x303A, 0x00 },
{ 0x303B, 0x10 },
{ 0x300D, 0x00 },
- { 0x034C, 0x10 },
- { 0x034D, 0x70 },
- { 0x034E, 0x0C },
- { 0x034F, 0x30 },
{ 0x0350, 0x01 },
{ 0x0204, 0x00 },
{ 0x0205, 0x00 },
@@ -266,234 +256,43 @@ static const struct imx258_reg mode_4208x3120_regs[] = {
{ 0x0220, 0x00 },
};
+static const struct imx258_reg mode_4208x3120_regs[] = {
+ { 0x0900, 0x00 },
+ { 0x0901, 0x11 },
+ { 0x0401, 0x00 },
+ { 0x0405, 0x10 },
+ { 0x040E, 0x0C },
+ { 0x040F, 0x30 },
+ { 0x034C, 0x10 },
+ { 0x034D, 0x70 },
+ { 0x034E, 0x0C },
+ { 0x034F, 0x30 },
+};
+
static const struct imx258_reg mode_2104_1560_regs[] = {
- { 0x0136, 0x13 },
- { 0x0137, 0x33 },
- { 0x3051, 0x00 },
- { 0x3052, 0x00 },
- { 0x4E21, 0x14 },
- { 0x6B11, 0xCF },
- { 0x7FF0, 0x08 },
- { 0x7FF1, 0x0F },
- { 0x7FF2, 0x08 },
- { 0x7FF3, 0x1B },
- { 0x7FF4, 0x23 },
- { 0x7FF5, 0x60 },
- { 0x7FF6, 0x00 },
- { 0x7FF7, 0x01 },
- { 0x7FF8, 0x00 },
- { 0x7FF9, 0x78 },
- { 0x7FFA, 0x00 },
- { 0x7FFB, 0x00 },
- { 0x7FFC, 0x00 },
- { 0x7FFD, 0x00 },
- { 0x7FFE, 0x00 },
- { 0x7FFF, 0x03 },
- { 0x7F76, 0x03 },
- { 0x7F77, 0xFE },
- { 0x7FA8, 0x03 },
- { 0x7FA9, 0xFE },
- { 0x7B24, 0x81 },
- { 0x7B25, 0x00 },
- { 0x6564, 0x07 },
- { 0x6B0D, 0x41 },
- { 0x653D, 0x04 },
- { 0x6B05, 0x8C },
- { 0x6B06, 0xF9 },
- { 0x6B08, 0x65 },
- { 0x6B09, 0xFC },
- { 0x6B0A, 0xCF },
- { 0x6B0B, 0xD2 },
- { 0x6700, 0x0E },
- { 0x6707, 0x0E },
- { 0x9104, 0x00 },
- { 0x4648, 0x7F },
- { 0x7420, 0x00 },
- { 0x7421, 0x1C },
- { 0x7422, 0x00 },
- { 0x7423, 0xD7 },
- { 0x5F04, 0x00 },
- { 0x5F05, 0xED },
- { 0x0112, 0x0A },
- { 0x0113, 0x0A },
- { 0x0114, 0x03 },
- { 0x0342, 0x14 },
- { 0x0343, 0xE8 },
- { 0x0344, 0x00 },
- { 0x0345, 0x00 },
- { 0x0346, 0x00 },
- { 0x0347, 0x00 },
- { 0x0348, 0x10 },
- { 0x0349, 0x6F },
- { 0x034A, 0x0C },
- { 0x034B, 0x2F },
- { 0x0381, 0x01 },
- { 0x0383, 0x01 },
- { 0x0385, 0x01 },
- { 0x0387, 0x01 },
{ 0x0900, 0x01 },
{ 0x0901, 0x12 },
{ 0x0401, 0x01 },
- { 0x0404, 0x00 },
{ 0x0405, 0x20 },
- { 0x0408, 0x00 },
- { 0x0409, 0x00 },
- { 0x040A, 0x00 },
- { 0x040B, 0x00 },
- { 0x040C, 0x10 },
- { 0x040D, 0x70 },
{ 0x040E, 0x06 },
{ 0x040F, 0x18 },
- { 0x3038, 0x00 },
- { 0x303A, 0x00 },
- { 0x303B, 0x10 },
- { 0x300D, 0x00 },
{ 0x034C, 0x08 },
{ 0x034D, 0x38 },
{ 0x034E, 0x06 },
{ 0x034F, 0x18 },
- { 0x0350, 0x01 },
- { 0x0204, 0x00 },
- { 0x0205, 0x00 },
- { 0x020E, 0x01 },
- { 0x020F, 0x00 },
- { 0x0210, 0x01 },
- { 0x0211, 0x00 },
- { 0x0212, 0x01 },
- { 0x0213, 0x00 },
- { 0x0214, 0x01 },
- { 0x0215, 0x00 },
- { 0x7BCD, 0x01 },
- { 0x94DC, 0x20 },
- { 0x94DD, 0x20 },
- { 0x94DE, 0x20 },
- { 0x95DC, 0x20 },
- { 0x95DD, 0x20 },
- { 0x95DE, 0x20 },
- { 0x7FB0, 0x00 },
- { 0x9010, 0x3E },
- { 0x9419, 0x50 },
- { 0x941B, 0x50 },
- { 0x9519, 0x50 },
- { 0x951B, 0x50 },
- { 0x3030, 0x00 },
- { 0x3032, 0x00 },
- { 0x0220, 0x00 },
};
static const struct imx258_reg mode_1048_780_regs[] = {
- { 0x0136, 0x13 },
- { 0x0137, 0x33 },
- { 0x3051, 0x00 },
- { 0x3052, 0x00 },
- { 0x4E21, 0x14 },
- { 0x6B11, 0xCF },
- { 0x7FF0, 0x08 },
- { 0x7FF1, 0x0F },
- { 0x7FF2, 0x08 },
- { 0x7FF3, 0x1B },
- { 0x7FF4, 0x23 },
- { 0x7FF5, 0x60 },
- { 0x7FF6, 0x00 },
- { 0x7FF7, 0x01 },
- { 0x7FF8, 0x00 },
- { 0x7FF9, 0x78 },
- { 0x7FFA, 0x00 },
- { 0x7FFB, 0x00 },
- { 0x7FFC, 0x00 },
- { 0x7FFD, 0x00 },
- { 0x7FFE, 0x00 },
- { 0x7FFF, 0x03 },
- { 0x7F76, 0x03 },
- { 0x7F77, 0xFE },
- { 0x7FA8, 0x03 },
- { 0x7FA9, 0xFE },
- { 0x7B24, 0x81 },
- { 0x7B25, 0x00 },
- { 0x6564, 0x07 },
- { 0x6B0D, 0x41 },
- { 0x653D, 0x04 },
- { 0x6B05, 0x8C },
- { 0x6B06, 0xF9 },
- { 0x6B08, 0x65 },
- { 0x6B09, 0xFC },
- { 0x6B0A, 0xCF },
- { 0x6B0B, 0xD2 },
- { 0x6700, 0x0E },
- { 0x6707, 0x0E },
- { 0x9104, 0x00 },
- { 0x4648, 0x7F },
- { 0x7420, 0x00 },
- { 0x7421, 0x1C },
- { 0x7422, 0x00 },
- { 0x7423, 0xD7 },
- { 0x5F04, 0x00 },
- { 0x5F05, 0xED },
- { 0x0112, 0x0A },
- { 0x0113, 0x0A },
- { 0x0114, 0x03 },
- { 0x0342, 0x14 },
- { 0x0343, 0xE8 },
- { 0x0344, 0x00 },
- { 0x0345, 0x00 },
- { 0x0346, 0x00 },
- { 0x0347, 0x00 },
- { 0x0348, 0x10 },
- { 0x0349, 0x6F },
- { 0x034A, 0x0C },
- { 0x034B, 0x2F },
- { 0x0381, 0x01 },
- { 0x0383, 0x01 },
- { 0x0385, 0x01 },
- { 0x0387, 0x01 },
{ 0x0900, 0x01 },
{ 0x0901, 0x14 },
{ 0x0401, 0x01 },
- { 0x0404, 0x00 },
{ 0x0405, 0x40 },
- { 0x0408, 0x00 },
- { 0x0409, 0x00 },
- { 0x040A, 0x00 },
- { 0x040B, 0x00 },
- { 0x040C, 0x10 },
- { 0x040D, 0x70 },
{ 0x040E, 0x03 },
{ 0x040F, 0x0C },
- { 0x3038, 0x00 },
- { 0x303A, 0x00 },
- { 0x303B, 0x10 },
- { 0x300D, 0x00 },
{ 0x034C, 0x04 },
{ 0x034D, 0x18 },
{ 0x034E, 0x03 },
{ 0x034F, 0x0C },
- { 0x0350, 0x01 },
- { 0x0204, 0x00 },
- { 0x0205, 0x00 },
- { 0x020E, 0x01 },
- { 0x020F, 0x00 },
- { 0x0210, 0x01 },
- { 0x0211, 0x00 },
- { 0x0212, 0x01 },
- { 0x0213, 0x00 },
- { 0x0214, 0x01 },
- { 0x0215, 0x00 },
- { 0x7BCD, 0x00 },
- { 0x94DC, 0x20 },
- { 0x94DD, 0x20 },
- { 0x94DE, 0x20 },
- { 0x95DC, 0x20 },
- { 0x95DD, 0x20 },
- { 0x95DE, 0x20 },
- { 0x7FB0, 0x00 },
- { 0x9010, 0x3E },
- { 0x9419, 0x50 },
- { 0x941B, 0x50 },
- { 0x9519, 0x50 },
- { 0x951B, 0x50 },
- { 0x3030, 0x00 },
- { 0x3032, 0x00 },
- { 0x0220, 0x00 },
};
static const char * const imx258_test_pattern_menu[] = {
@@ -955,6 +754,13 @@ static int imx258_start_streaming(struct imx258 *imx258)
return ret;
}
+ ret = imx258_write_regs(imx258, mode_common_regs,
+ ARRAY_SIZE(mode_common_regs));
+ if (ret) {
+ dev_err(&client->dev, "%s failed to set common regs\n", __func__);
+ return ret;
+ }
+
/* Apply default values of current mode */
reg_list = &imx258->cur_mode->reg_list;
ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs);
--
2.42.0
From: Dave Stevenson <[email protected]>
Libcamera requires the cropping information for each mode, so
add this information to the driver.
Signed-off-by: Dave Stevenson <[email protected]>
Signed-off-by: Luigi311 <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
drivers/media/i2c/imx258.c | 90 ++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 1f5fb980cfbe..979ac7872249 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -77,6 +77,14 @@
#define REG_CONFIG_MIRROR_FLIP 0x03
#define REG_CONFIG_FLIP_TEST_PATTERN 0x02
+/* IMX258 native and active pixel array size. */
+#define IMX258_NATIVE_WIDTH 4224U
+#define IMX258_NATIVE_HEIGHT 3192U
+#define IMX258_PIXEL_ARRAY_LEFT 8U
+#define IMX258_PIXEL_ARRAY_TOP 16U
+#define IMX258_PIXEL_ARRAY_WIDTH 4208U
+#define IMX258_PIXEL_ARRAY_HEIGHT 3120U
+
struct imx258_reg {
u16 address;
u8 val;
@@ -116,6 +124,9 @@ struct imx258_mode {
u32 link_freq_index;
/* Default register values */
struct imx258_reg_list reg_list;
+
+ /* Analog crop rectangle. */
+ struct v4l2_rect crop;
};
/*
@@ -567,6 +578,12 @@ static const struct imx258_mode supported_modes[] = {
.regs = mode_4208x3120_regs,
},
.link_freq_index = IMX258_LINK_FREQ_1267MBPS,
+ .crop = {
+ .left = IMX258_PIXEL_ARRAY_LEFT,
+ .top = IMX258_PIXEL_ARRAY_TOP,
+ .width = 4208,
+ .height = 3120,
+ },
},
{
.width = 2104,
@@ -578,6 +595,12 @@ static const struct imx258_mode supported_modes[] = {
.regs = mode_2104_1560_regs,
},
.link_freq_index = IMX258_LINK_FREQ_640MBPS,
+ .crop = {
+ .left = IMX258_PIXEL_ARRAY_LEFT,
+ .top = IMX258_PIXEL_ARRAY_TOP,
+ .width = 4208,
+ .height = 3120,
+ },
},
{
.width = 1048,
@@ -589,6 +612,12 @@ static const struct imx258_mode supported_modes[] = {
.regs = mode_1048_780_regs,
},
.link_freq_index = IMX258_LINK_FREQ_640MBPS,
+ .crop = {
+ .left = IMX258_PIXEL_ARRAY_LEFT,
+ .top = IMX258_PIXEL_ARRAY_TOP,
+ .width = 4208,
+ .height = 3120,
+ },
},
};
@@ -705,6 +734,7 @@ static int imx258_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
{
struct v4l2_mbus_framefmt *try_fmt =
v4l2_subdev_state_get_format(fh->state, 0);
+ struct v4l2_rect *try_crop;
/* Initialize try_fmt */
try_fmt->width = supported_modes[0].width;
@@ -712,6 +742,13 @@ static int imx258_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
try_fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
try_fmt->field = V4L2_FIELD_NONE;
+ /* Initialize try_crop */
+ try_crop = v4l2_subdev_state_get_crop(fh->state, 0);
+ try_crop->left = IMX258_PIXEL_ARRAY_LEFT;
+ try_crop->top = IMX258_PIXEL_ARRAY_TOP;
+ try_crop->width = IMX258_PIXEL_ARRAY_WIDTH;
+ try_crop->height = IMX258_PIXEL_ARRAY_HEIGHT;
+
return 0;
}
@@ -959,6 +996,58 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
return 0;
}
+static const struct v4l2_rect *
+__imx258_get_pad_crop(struct imx258 *imx258,
+ struct v4l2_subdev_state *sd_state,
+ unsigned int pad, enum v4l2_subdev_format_whence which)
+{
+ switch (which) {
+ case V4L2_SUBDEV_FORMAT_TRY:
+ return v4l2_subdev_state_get_crop(sd_state, pad);
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ return &imx258->cur_mode->crop;
+ }
+
+ return NULL;
+}
+
+static int imx258_get_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_selection *sel)
+{
+ switch (sel->target) {
+ case V4L2_SEL_TGT_CROP: {
+ struct imx258 *imx258 = to_imx258(sd);
+
+ mutex_lock(&imx258->mutex);
+ sel->r = *__imx258_get_pad_crop(imx258, sd_state, sel->pad,
+ sel->which);
+ mutex_unlock(&imx258->mutex);
+
+ return 0;
+ }
+
+ case V4L2_SEL_TGT_NATIVE_SIZE:
+ sel->r.left = 0;
+ sel->r.top = 0;
+ sel->r.width = IMX258_NATIVE_WIDTH;
+ sel->r.height = IMX258_NATIVE_HEIGHT;
+
+ return 0;
+
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ sel->r.left = IMX258_PIXEL_ARRAY_LEFT;
+ sel->r.top = IMX258_PIXEL_ARRAY_TOP;
+ sel->r.width = IMX258_PIXEL_ARRAY_WIDTH;
+ sel->r.height = IMX258_PIXEL_ARRAY_HEIGHT;
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
/* Start streaming */
static int imx258_start_streaming(struct imx258 *imx258)
{
@@ -1135,6 +1224,7 @@ static const struct v4l2_subdev_pad_ops imx258_pad_ops = {
.get_fmt = imx258_get_pad_format,
.set_fmt = imx258_set_pad_format,
.enum_frame_size = imx258_enum_frame_size,
+ .get_selection = imx258_get_selection,
};
static const struct v4l2_subdev_ops imx258_subdev_ops = {
--
2.42.0
From: Dave Stevenson <[email protected]>
The data sheet states that the maximum value for registers
0x0340/0x0341 FRM_LENGTH_LINES is 65525(decimal), not the
0xFFFF defined in this driver. Correct this limit.
Signed-off-by: Dave Stevenson <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
drivers/media/i2c/imx258.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 85c2f1ccaea1..c2c5e819ddc0 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -28,7 +28,7 @@
#define IMX258_VTS_30FPS 0x0c50
#define IMX258_VTS_30FPS_2K 0x0638
#define IMX258_VTS_30FPS_VGA 0x034c
-#define IMX258_VTS_MAX 0xffff
+#define IMX258_VTS_MAX 65525
#define IMX258_REG_VTS 0x0340
--
2.42.0
From: Luigi311 <[email protected]>
On some boards powerdown signal needs to be deasserted for this
sensor to be enabled.
Signed-off-by: Ondrej Jirman <[email protected]>
---
.../devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++
drivers/media/i2c/imx258.c | 13 +++++++++++++
2 files changed, 17 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
index c7856de15ba3..0414085bf22f 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
@@ -35,6 +35,10 @@ properties:
reg:
maxItems: 1
+ powerdown-gpios:
+ description: |-
+ Reference to the GPIO connected to the PWDN pin, if any.
+
reset-gpios:
description: |-
Reference to the GPIO connected to the XCLR pin, if any.
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index c559a06bf180..d8c51d5f04e0 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -686,6 +686,8 @@ struct imx258 {
unsigned int lane_mode_idx;
unsigned int csi2_flags;
+ struct gpio_desc *powerdown_gpio;
+
/*
* Mutex for serialized access:
* Protect sensor module set pad format and start/stop streaming safely.
@@ -1220,6 +1222,8 @@ static int imx258_power_on(struct device *dev)
struct imx258 *imx258 = to_imx258(sd);
int ret;
+ gpiod_set_value_cansleep(imx258->powerdown_gpio, 0);
+
ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
imx258->supplies);
if (ret) {
@@ -1231,6 +1235,7 @@ static int imx258_power_on(struct device *dev)
ret = clk_prepare_enable(imx258->clk);
if (ret) {
dev_err(dev, "failed to enable clock\n");
+ gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
}
@@ -1245,6 +1250,8 @@ static int imx258_power_off(struct device *dev)
clk_disable_unprepare(imx258->clk);
regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
+ gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
+
return 0;
}
@@ -1548,6 +1555,12 @@ static int imx258_probe(struct i2c_client *client)
if (!imx258->variant_cfg)
imx258->variant_cfg = &imx258_cfg;
+ /* request optional power down pin */
+ imx258->powerdown_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(imx258->powerdown_gpio))
+ return PTR_ERR(imx258->powerdown_gpio);
+
/* Initialize subdev */
v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
--
2.42.0
From: Luigi311 <[email protected]>
Use understandable macros instead of raw values.
Signed-off-by: Ondrej Jirman <[email protected]>
Signed-off-by: Luigi311 <[email protected]>
---
drivers/media/i2c/imx258.c | 434 ++++++++++++++++++-------------------
1 file changed, 207 insertions(+), 227 deletions(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 52eaeeae1bed..c559a06bf180 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -33,8 +33,6 @@
#define IMX258_VTS_30FPS_VGA 0x034c
#define IMX258_VTS_MAX 65525
-#define IMX258_REG_VTS 0x0340
-
/* HBLANK control - read only */
#define IMX258_PPL_DEFAULT 5352
@@ -90,6 +88,53 @@
#define IMX258_PIXEL_ARRAY_WIDTH 4208U
#define IMX258_PIXEL_ARRAY_HEIGHT 3120U
+/* regs */
+#define IMX258_REG_PLL_MULT_DRIV 0x0310
+#define IMX258_REG_IVTPXCK_DIV 0x0301
+#define IMX258_REG_IVTSYCK_DIV 0x0303
+#define IMX258_REG_PREPLLCK_VT_DIV 0x0305
+#define IMX258_REG_IOPPXCK_DIV 0x0309
+#define IMX258_REG_IOPSYCK_DIV 0x030b
+#define IMX258_REG_PREPLLCK_OP_DIV 0x030d
+#define IMX258_REG_PHASE_PIX_OUTEN 0x3030
+#define IMX258_REG_PDPIX_DATA_RATE 0x3032
+#define IMX258_REG_SCALE_MODE 0x0401
+#define IMX258_REG_SCALE_MODE_EXT 0x3038
+#define IMX258_REG_AF_WINDOW_MODE 0x7bcd
+#define IMX258_REG_FRM_LENGTH_CTL 0x0350
+#define IMX258_REG_CSI_LANE_MODE 0x0114
+#define IMX258_REG_X_EVN_INC 0x0381
+#define IMX258_REG_X_ODD_INC 0x0383
+#define IMX258_REG_Y_EVN_INC 0x0385
+#define IMX258_REG_Y_ODD_INC 0x0387
+#define IMX258_REG_BINNING_MODE 0x0900
+#define IMX258_REG_BINNING_TYPE_V 0x0901
+#define IMX258_REG_FORCE_FD_SUM 0x300d
+#define IMX258_REG_DIG_CROP_X_OFFSET 0x0408
+#define IMX258_REG_DIG_CROP_Y_OFFSET 0x040a
+#define IMX258_REG_DIG_CROP_IMAGE_WIDTH 0x040c
+#define IMX258_REG_DIG_CROP_IMAGE_HEIGHT 0x040e
+#define IMX258_REG_SCALE_M 0x0404
+#define IMX258_REG_X_OUT_SIZE 0x034c
+#define IMX258_REG_Y_OUT_SIZE 0x034e
+#define IMX258_REG_X_ADD_STA 0x0344
+#define IMX258_REG_Y_ADD_STA 0x0346
+#define IMX258_REG_X_ADD_END 0x0348
+#define IMX258_REG_Y_ADD_END 0x034a
+#define IMX258_REG_EXCK_FREQ 0x0136
+#define IMX258_REG_CSI_DT_FMT 0x0112
+#define IMX258_REG_LINE_LENGTH_PCK 0x0342
+#define IMX258_REG_SCALE_M_EXT 0x303a
+#define IMX258_REG_FRM_LENGTH_LINES 0x0340
+#define IMX258_REG_FINE_INTEG_TIME 0x0200
+#define IMX258_REG_PLL_IVT_MPY 0x0306
+#define IMX258_REG_PLL_IOP_MPY 0x030e
+#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H 0x0820
+#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L 0x0822
+
+#define REG8(a, v) { a, v }
+#define REG16(a, v) { a, ((v) >> 8) & 0xff }, { (a) + 1, (v) & 0xff }
+
struct imx258_reg {
u16 address;
u8 val;
@@ -145,179 +190,139 @@ struct imx258_mode {
* lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
*/
static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
- { 0x0136, 0x13 },
- { 0x0137, 0x33 },
- { 0x0301, 0x0A },
- { 0x0303, 0x02 },
- { 0x0305, 0x03 },
- { 0x0306, 0x00 },
- { 0x0307, 0xC6 },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x01 },
- { 0x0820, 0x09 },
- { 0x0821, 0xa6 },
- { 0x0822, 0x66 },
- { 0x0823, 0x66 },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1333),
+ REG8(IMX258_REG_IVTPXCK_DIV, 10),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x00C6),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 1),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x09A6),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x6666),
};
static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
- { 0x0136, 0x13 },
- { 0x0137, 0x33 },
- { 0x0301, 0x05 },
- { 0x0303, 0x02 },
- { 0x0305, 0x03 },
- { 0x0306, 0x00 },
- { 0x0307, 0xC6 },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x03 },
- { 0x0820, 0x13 },
- { 0x0821, 0x4C },
- { 0x0822, 0xCC },
- { 0x0823, 0xCC },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1333),
+ REG8(IMX258_REG_IVTPXCK_DIV, 5),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x00C6),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 3),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x134C),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0xCCCC),
};
static const struct imx258_reg mipi_1272mbps_24mhz_2l[] = {
- { 0x0136, 0x18 },
- { 0x0137, 0x00 },
- { 0x0301, 0x0a },
- { 0x0303, 0x02 },
- { 0x0305, 0x04 },
- { 0x0306, 0x00 },
- { 0x0307, 0xD4 },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x01 },
- { 0x0820, 0x13 },
- { 0x0821, 0x4C },
- { 0x0822, 0xCC },
- { 0x0823, 0xCC },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1800),
+ REG8(IMX258_REG_IVTPXCK_DIV, 10),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x00D4),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 1),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x134C),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0xCCCC),
};
static const struct imx258_reg mipi_1272mbps_24mhz_4l[] = {
- { 0x0136, 0x18 },
- { 0x0137, 0x00 },
- { 0x0301, 0x05 },
- { 0x0303, 0x02 },
- { 0x0305, 0x04 },
- { 0x0306, 0x00 },
- { 0x0307, 0xD4 },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x03 },
- { 0x0820, 0x13 },
- { 0x0821, 0xE0 },
- { 0x0822, 0x00 },
- { 0x0823, 0x00 },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1800),
+ REG8(IMX258_REG_IVTPXCK_DIV, 5),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x00D4),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 3),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x13E0),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
};
static const struct imx258_reg mipi_640mbps_19_2mhz_2l[] = {
- { 0x0136, 0x13 },
- { 0x0137, 0x33 },
- { 0x0301, 0x05 },
- { 0x0303, 0x02 },
- { 0x0305, 0x03 },
- { 0x0306, 0x00 },
- { 0x0307, 0x64 },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x01 },
- { 0x0820, 0x05 },
- { 0x0821, 0x00 },
- { 0x0822, 0x00 },
- { 0x0823, 0x00 },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1333),
+ REG8(IMX258_REG_IVTPXCK_DIV, 5),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x0064),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 1),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0500),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
};
static const struct imx258_reg mipi_640mbps_19_2mhz_4l[] = {
- { 0x0136, 0x13 },
- { 0x0137, 0x33 },
- { 0x0301, 0x05 },
- { 0x0303, 0x02 },
- { 0x0305, 0x03 },
- { 0x0306, 0x00 },
- { 0x0307, 0x64 },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x03 },
- { 0x0820, 0x0A },
- { 0x0821, 0x00 },
- { 0x0822, 0x00 },
- { 0x0823, 0x00 },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1333),
+ REG8(IMX258_REG_IVTPXCK_DIV, 5),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x0064),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 3),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
};
static const struct imx258_reg mipi_642mbps_24mhz_2l[] = {
- { 0x0136, 0x18 },
- { 0x0137, 0x00 },
- { 0x0301, 0x05 },
- { 0x0303, 0x02 },
- { 0x0305, 0x04 },
- { 0x0306, 0x00 },
- { 0x0307, 0x6B },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x01 },
- { 0x0820, 0x0A },
- { 0x0821, 0x00 },
- { 0x0822, 0x00 },
- { 0x0823, 0x00 },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1800),
+ REG8(IMX258_REG_IVTPXCK_DIV, 5),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x006B),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 1),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
};
static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
- { 0x0136, 0x18 },
- { 0x0137, 0x00 },
- { 0x0301, 0x05 },
- { 0x0303, 0x02 },
- { 0x0305, 0x04 },
- { 0x0306, 0x00 },
- { 0x0307, 0x6B },
- { 0x0309, 0x0A },
- { 0x030B, 0x01 },
- { 0x030D, 0x02 },
- { 0x030E, 0x00 },
- { 0x030F, 0xD8 },
- { 0x0310, 0x00 },
-
- { 0x0114, 0x03 },
- { 0x0820, 0x0A },
- { 0x0821, 0x00 },
- { 0x0822, 0x00 },
- { 0x0823, 0x00 },
+ REG16(IMX258_REG_EXCK_FREQ, 0x1800),
+ REG8(IMX258_REG_IVTPXCK_DIV, 5),
+ REG8(IMX258_REG_IVTSYCK_DIV, 2),
+ REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
+ REG16(IMX258_REG_PLL_IVT_MPY, 0x006B),
+ REG8(IMX258_REG_IOPPXCK_DIV, 10),
+ REG8(IMX258_REG_IOPSYCK_DIV, 1),
+ REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
+ REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
+ REG8(IMX258_REG_PLL_MULT_DRIV, 0),
+
+ REG8(IMX258_REG_CSI_LANE_MODE, 3),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
+ REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
};
static const struct imx258_reg mode_common_regs[] = {
@@ -363,45 +368,29 @@ static const struct imx258_reg mode_common_regs[] = {
{ 0x7423, 0xD7 },
{ 0x5F04, 0x00 },
{ 0x5F05, 0xED },
- { 0x0112, 0x0A },
- { 0x0113, 0x0A },
- { 0x0342, 0x14 },
- { 0x0343, 0xE8 },
- { 0x0344, 0x00 },
- { 0x0345, 0x00 },
- { 0x0346, 0x00 },
- { 0x0347, 0x00 },
- { 0x0348, 0x10 },
- { 0x0349, 0x6F },
- { 0x034A, 0x0C },
- { 0x034B, 0x2F },
- { 0x0381, 0x01 },
- { 0x0383, 0x01 },
- { 0x0385, 0x01 },
- { 0x0387, 0x01 },
- { 0x0404, 0x00 },
- { 0x0408, 0x00 },
- { 0x0409, 0x00 },
- { 0x040A, 0x00 },
- { 0x040B, 0x00 },
- { 0x040C, 0x10 },
- { 0x040D, 0x70 },
- { 0x3038, 0x00 },
- { 0x303A, 0x00 },
- { 0x303B, 0x10 },
- { 0x300D, 0x00 },
- { 0x0350, 0x00 },
- { 0x0204, 0x00 },
- { 0x0205, 0x00 },
- { 0x020E, 0x01 },
- { 0x020F, 0x00 },
- { 0x0210, 0x01 },
- { 0x0211, 0x00 },
- { 0x0212, 0x01 },
- { 0x0213, 0x00 },
- { 0x0214, 0x01 },
- { 0x0215, 0x00 },
- { 0x7BCD, 0x00 },
+ REG16(IMX258_REG_CSI_DT_FMT, 0x0a0a),
+ REG16(IMX258_REG_LINE_LENGTH_PCK, 5352 ),
+ REG16(IMX258_REG_X_ADD_STA, 0),
+ REG16(IMX258_REG_Y_ADD_STA, 0),
+ REG16(IMX258_REG_X_ADD_END, 4207),
+ REG16(IMX258_REG_Y_ADD_END, 3119),
+ REG8(IMX258_REG_X_EVN_INC, 1),
+ REG8(IMX258_REG_X_ODD_INC, 1),
+ REG8(IMX258_REG_Y_EVN_INC, 1),
+ REG8(IMX258_REG_Y_ODD_INC, 1),
+ REG16(IMX258_REG_DIG_CROP_X_OFFSET, 0),
+ REG16(IMX258_REG_DIG_CROP_Y_OFFSET, 0),
+ REG16(IMX258_REG_DIG_CROP_IMAGE_WIDTH, 4208),
+ REG8(IMX258_REG_SCALE_MODE_EXT, 0),
+ REG16(IMX258_REG_SCALE_M_EXT, 16),
+ REG8(IMX258_REG_FORCE_FD_SUM, 0),
+ REG8(IMX258_REG_FRM_LENGTH_CTL, 0),
+ REG16(IMX258_REG_ANALOG_GAIN, 0),
+ REG16(IMX258_REG_GR_DIGITAL_GAIN, 256),
+ REG16(IMX258_REG_R_DIGITAL_GAIN, 256),
+ REG16(IMX258_REG_B_DIGITAL_GAIN, 256),
+ REG16(IMX258_REG_GB_DIGITAL_GAIN, 256),
+ REG8(IMX258_REG_AF_WINDOW_MODE, 0),
{ 0x94DC, 0x20 },
{ 0x94DD, 0x20 },
{ 0x94DE, 0x20 },
@@ -414,48 +403,39 @@ static const struct imx258_reg mode_common_regs[] = {
{ 0x941B, 0x50 },
{ 0x9519, 0x50 },
{ 0x951B, 0x50 },
- { 0x3030, 0x00 },
- { 0x3032, 0x00 },
- { 0x0220, 0x00 },
+ REG8(IMX258_REG_PHASE_PIX_OUTEN, 0),
+ REG8(IMX258_REG_PDPIX_DATA_RATE, 0),
+ REG8(IMX258_REG_HDR, 0),
};
static const struct imx258_reg mode_4208x3120_regs[] = {
- { 0x0900, 0x00 },
- { 0x0901, 0x11 },
- { 0x0401, 0x00 },
- { 0x0405, 0x10 },
- { 0x040E, 0x0C },
- { 0x040F, 0x30 },
- { 0x034C, 0x10 },
- { 0x034D, 0x70 },
- { 0x034E, 0x0C },
- { 0x034F, 0x30 },
+ REG8(IMX258_REG_BINNING_MODE, 0),
+ REG8(IMX258_REG_BINNING_TYPE_V, 0x11),
+ REG8(IMX258_REG_SCALE_MODE, 0),
+ REG16(IMX258_REG_SCALE_M, 16),
+ REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 3120),
+ REG16(IMX258_REG_X_OUT_SIZE, 4208),
+ REG16(IMX258_REG_Y_OUT_SIZE, 3120),
};
static const struct imx258_reg mode_2104_1560_regs[] = {
- { 0x0900, 0x01 },
- { 0x0901, 0x12 },
- { 0x0401, 0x01 },
- { 0x0405, 0x20 },
- { 0x040E, 0x06 },
- { 0x040F, 0x18 },
- { 0x034C, 0x08 },
- { 0x034D, 0x38 },
- { 0x034E, 0x06 },
- { 0x034F, 0x18 },
+ REG8(IMX258_REG_BINNING_MODE, 1),
+ REG8(IMX258_REG_BINNING_TYPE_V, 0x12),
+ REG8(IMX258_REG_SCALE_MODE, 1),
+ REG16(IMX258_REG_SCALE_M, 32),
+ REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 1560),
+ REG16(IMX258_REG_X_OUT_SIZE, 2104),
+ REG16(IMX258_REG_Y_OUT_SIZE, 1560),
};
static const struct imx258_reg mode_1048_780_regs[] = {
- { 0x0900, 0x01 },
- { 0x0901, 0x14 },
- { 0x0401, 0x01 },
- { 0x0405, 0x40 },
- { 0x040E, 0x03 },
- { 0x040F, 0x0C },
- { 0x034C, 0x04 },
- { 0x034D, 0x18 },
- { 0x034E, 0x03 },
- { 0x034F, 0x0C },
+ REG8(IMX258_REG_BINNING_MODE, 1),
+ REG8(IMX258_REG_BINNING_TYPE_V, 0x14),
+ REG8(IMX258_REG_SCALE_MODE, 1),
+ REG16(IMX258_REG_SCALE_M, 64),
+ REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 780),
+ REG16(IMX258_REG_X_OUT_SIZE, 1048),
+ REG16(IMX258_REG_Y_OUT_SIZE, 780),
};
struct imx258_variant_cfg {
@@ -930,7 +910,7 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
}
break;
case V4L2_CID_VBLANK:
- ret = imx258_write_reg(imx258, IMX258_REG_VTS,
+ ret = imx258_write_reg(imx258, IMX258_REG_FRM_LENGTH_LINES,
IMX258_REG_VALUE_16BIT,
imx258->cur_mode->height + ctrl->val);
break;
--
2.42.0
From: Dave Stevenson <[email protected]>
There are a number of variants of the imx258 modules that can not
be differentiated at runtime, so add compatible strings for them.
Signed-off-by: Dave Stevenson <[email protected]>
Signed-off-by: Luigi311 <[email protected]>
---
.../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
index bee61a443b23..c7856de15ba3 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
@@ -14,10 +14,14 @@ description: |-
type stacked image sensor with a square pixel array of size 4208 x 3120. It
is programmable through I2C interface. Image data is sent through MIPI
CSI-2.
+ There are a number of variants of the sensor which cannot be detected at
+ runtime, so multiple compatible strings are required to differentiate these.
properties:
compatible:
- const: sony,imx258
+ - enum:
+ - sony,imx258
+ - sony,imx258-pdaf
assigned-clocks: true
assigned-clock-parents: true
--
2.42.0
From: Luigi311 <[email protected]>
It was documented in DT, but not implemented.
Signed-off-by: Ondrej Jirman <[email protected]>
---
drivers/media/i2c/imx258.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index d8c51d5f04e0..42e1c9246bed 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -687,6 +687,7 @@ struct imx258 {
unsigned int csi2_flags;
struct gpio_desc *powerdown_gpio;
+ struct gpio_desc *reset_gpio;
/*
* Mutex for serialized access:
@@ -1239,7 +1240,11 @@ static int imx258_power_on(struct device *dev)
regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
}
- return ret;
+ gpiod_set_value_cansleep(imx258->reset_gpio, 0);
+
+ usleep_range(400, 500);
+
+ return 0;
}
static int imx258_power_off(struct device *dev)
@@ -1250,6 +1255,7 @@ static int imx258_power_off(struct device *dev)
clk_disable_unprepare(imx258->clk);
regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
+ gpiod_set_value_cansleep(imx258->reset_gpio, 1);
gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
return 0;
@@ -1561,6 +1567,12 @@ static int imx258_probe(struct i2c_client *client)
if (IS_ERR(imx258->powerdown_gpio))
return PTR_ERR(imx258->powerdown_gpio);
+ /* request optional reset pin */
+ imx258->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(imx258->reset_gpio))
+ return PTR_ERR(imx258->reset_gpio);
+
/* Initialize subdev */
v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
--
2.42.0
From: Dave Stevenson <[email protected]>
Whilst not documented, register 0x0103 bit 0 is the soft
reset for the sensor, so send it before trying to configure
the sensor.
Signed-off-by: Dave Stevenson <[email protected]>
---
drivers/media/i2c/imx258.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index c2c5e819ddc0..a62ed8c26663 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -20,6 +20,8 @@
#define IMX258_MODE_STANDBY 0x00
#define IMX258_MODE_STREAMING 0x01
+#define IMX258_REG_RESET 0x0103
+
/* Chip ID */
#define IMX258_REG_CHIP_ID 0x0016
#define IMX258_CHIP_ID 0x0258
@@ -1059,6 +1061,16 @@ static int imx258_start_streaming(struct imx258 *imx258)
const struct imx258_link_freq_config *link_freq_cfg;
int ret, link_freq_index;
+ ret = imx258_write_reg(imx258, IMX258_REG_RESET, IMX258_REG_VALUE_08BIT,
+ 0x01);
+ if (ret) {
+ dev_err(&client->dev, "%s failed to reset sensor\n", __func__);
+ return ret;
+ }
+
+ /* 12ms is required from poweron to standby */
+ fsleep(12000);
+
/* Setup PLL */
link_freq_index = imx258->cur_mode->link_freq_index;
link_freq_cfg = &imx258->link_freq_configs[link_freq_index];
--
2.42.0
From: Dave Stevenson <[email protected]>
imx258.yaml doesn't include the vendor prefix of sony, so
rename to add it.
Update the id entry and MAINTAINERS to match.
Signed-off-by: Dave Stevenson <[email protected]>
Acked-by: Conor Dooley <[email protected]>
---
.../bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} | 2 +-
MAINTAINERS | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (97%)
diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
similarity index 97%
rename from Documentation/devicetree/bindings/media/i2c/imx258.yaml
rename to Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
index 80d24220baa0..bee61a443b23 100644
--- a/Documentation/devicetree/bindings/media/i2c/imx258.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
-$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
+$id: http://devicetree.org/schemas/media/i2c/sony,imx258.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
diff --git a/MAINTAINERS b/MAINTAINERS
index aa3b947fb080..1f17f6734bf5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20464,7 +20464,7 @@ M: Sakari Ailus <[email protected]>
L: [email protected]
S: Maintained
T: git git://linuxtv.org/media_tree.git
-F: Documentation/devicetree/bindings/media/i2c/imx258.yaml
+F: Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
F: drivers/media/i2c/imx258.c
SONY IMX274 SENSOR DRIVER
--
2.42.0
On Wed, Mar 27, 2024 at 05:17:03PM -0600, [email protected] wrote:
> From: Dave Stevenson <[email protected]>
>
> imx258.yaml doesn't include the vendor prefix of sony, so
> rename to add it.
> Update the id entry and MAINTAINERS to match.
>
> Signed-off-by: Dave Stevenson <[email protected]>
> Acked-by: Conor Dooley <[email protected]>
This is a v1 with my ack, something has gone awry here. It's also
missing your signoff. Did you pick up someone else's series?
> ---
> .../bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} | 2 +-
> MAINTAINERS | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (97%)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> similarity index 97%
> rename from Documentation/devicetree/bindings/media/i2c/imx258.yaml
> rename to Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> index 80d24220baa0..bee61a443b23 100644
> --- a/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> %YAML 1.2
> ---
> -$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx258.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aa3b947fb080..1f17f6734bf5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20464,7 +20464,7 @@ M: Sakari Ailus <[email protected]>
> L: [email protected]
> S: Maintained
> T: git git://linuxtv.org/media_tree.git
> -F: Documentation/devicetree/bindings/media/i2c/imx258.yaml
> +F: Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> F: drivers/media/i2c/imx258.c
>
> SONY IMX274 SENSOR DRIVER
> --
> 2.42.0
>
On Wed, 27 Mar 2024 17:17:04 -0600, [email protected] wrote:
> From: Dave Stevenson <[email protected]>
>
> There are a number of variants of the imx258 modules that can not
> be differentiated at runtime, so add compatible strings for them.
>
> Signed-off-by: Dave Stevenson <[email protected]>
> Signed-off-by: Luigi311 <[email protected]>
> ---
> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: ignoring, error in schema: properties: compatible
Documentation/devicetree/bindings/media/i2c/sony,imx258.example.dtb: /example-0/i2c/sensor@6c: failed to match any schema with compatible: ['sony,imx258']
Documentation/devicetree/bindings/media/i2c/sony,imx258.example.dtb: /example-1/i2c/sensor@6c: failed to match any schema with compatible: ['sony,imx258']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 3/27/24 17:47, Conor Dooley wrote:
> On Wed, Mar 27, 2024 at 05:17:03PM -0600, [email protected] wrote:
>> From: Dave Stevenson <[email protected]>
>>
>> imx258.yaml doesn't include the vendor prefix of sony, so
>> rename to add it.
>> Update the id entry and MAINTAINERS to match.
>>
>> Signed-off-by: Dave Stevenson <[email protected]>
>> Acked-by: Conor Dooley <[email protected]>
>
> This is a v1 with my ack, something has gone awry here. It's also
> missing your signoff. Did you pick up someone else's series?
Yes, this is a continuation of Dave's work. I contacted him directly,
and he mentioned that he is unable to submit a v2 any time soon and
was open to someone else continuing it in his stead. This is my first
time submitting a patch via a mailing list, so I'm not sure if I'm
missing something, but I only added my sign off for anything that
actually included work from my side and not just bringing his patch
forward to this patch series.
>
>> ---
>> .../bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} | 2 +-
>> MAINTAINERS | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>> rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (97%)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> similarity index 97%
>> rename from Documentation/devicetree/bindings/media/i2c/imx258.yaml
>> rename to Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> index 80d24220baa0..bee61a443b23 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/imx258.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> @@ -1,7 +1,7 @@
>> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> %YAML 1.2
>> ---
>> -$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
>> +$id: http://devicetree.org/schemas/media/i2c/sony,imx258.yaml#
>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>> title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index aa3b947fb080..1f17f6734bf5 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -20464,7 +20464,7 @@ M: Sakari Ailus <[email protected]>
>> L: [email protected]
>> S: Maintained
>> T: git git://linuxtv.org/media_tree.git
>> -F: Documentation/devicetree/bindings/media/i2c/imx258.yaml
>> +F: Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> F: drivers/media/i2c/imx258.c
>>
>> SONY IMX274 SENSOR DRIVER
>> --
>> 2.42.0
>>
On 28/03/2024 00:17, [email protected] wrote:
> From: Dave Stevenson <[email protected]>
>
> There are a number of variants of the imx258 modules that can not
> be differentiated at runtime, so add compatible strings for them.
>
> Signed-off-by: Dave Stevenson <[email protected]>
> Signed-off-by: Luigi311 <[email protected]>
> ---
> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> index bee61a443b23..c7856de15ba3 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> @@ -14,10 +14,14 @@ description: |-
> type stacked image sensor with a square pixel array of size 4208 x 3120. It
> is programmable through I2C interface. Image data is sent through MIPI
> CSI-2.
> + There are a number of variants of the sensor which cannot be detected at
> + runtime, so multiple compatible strings are required to differentiate these.
>
> properties:
> compatible:
> - const: sony,imx258
> + - enum:
> + - sony,imx258
Two people working on patch but no one tested it before sending. Do not
send untested code.
It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.
Best regards,
Krzysztof
On 28/03/2024 00:16, [email protected] wrote:
> From: Luigi311 <[email protected]>
>
> Resend due to email message limits being exceeded.
>
> v2 changes:
Please mark your patches correctly. Use b4 or -v2 for format-patch.
Best regards,
Krzysztof
Quoting [email protected] (2024-03-28 00:57:34)
> On 3/27/24 17:47, Conor Dooley wrote:
> > On Wed, Mar 27, 2024 at 05:17:03PM -0600, [email protected] wrote:
> >> From: Dave Stevenson <[email protected]>
> >>
> >> imx258.yaml doesn't include the vendor prefix of sony, so
> >> rename to add it.
> >> Update the id entry and MAINTAINERS to match.
> >>
> >> Signed-off-by: Dave Stevenson <[email protected]>
> >> Acked-by: Conor Dooley <[email protected]>
> >
> > This is a v1 with my ack, something has gone awry here. It's also
> > missing your signoff. Did you pick up someone else's series?
>
> Yes, this is a continuation of Dave's work. I contacted him directly,
> and he mentioned that he is unable to submit a v2 any time soon and
> was open to someone else continuing it in his stead. This is my first
> time submitting a patch via a mailing list, so I'm not sure if I'm
> missing something, but I only added my sign off for anything that
> actually included work from my side and not just bringing his patch
> forward to this patch series.
Your cover letter states v2, but the individual patches do not.
Add the '-v2' (or, rather, next it will be '-v3') to git format-patch
when you save your series and it will add the version to each patch. You
can also add '-s' to that command I believe to add your SoB to each
patch.
--
Kieran
>
> >
> >> ---
> >> .../bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} | 2 +-
> >> MAINTAINERS | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >> rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (97%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> similarity index 97%
> >> rename from Documentation/devicetree/bindings/media/i2c/imx258.yaml
> >> rename to Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> index 80d24220baa0..bee61a443b23 100644
> >> --- a/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> >> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> @@ -1,7 +1,7 @@
> >> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> %YAML 1.2
> >> ---
> >> -$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
> >> +$id: http://devicetree.org/schemas/media/i2c/sony,imx258.yaml#
> >> $schema: http://devicetree.org/meta-schemas/core.yaml#
> >>
> >> title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index aa3b947fb080..1f17f6734bf5 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -20464,7 +20464,7 @@ M: Sakari Ailus <[email protected]>
> >> L: [email protected]
> >> S: Maintained
> >> T: git git://linuxtv.org/media_tree.git
> >> -F: Documentation/devicetree/bindings/media/i2c/imx258.yaml
> >> +F: Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> F: drivers/media/i2c/imx258.c
> >>
> >> SONY IMX274 SENSOR DRIVER
> >> --
> >> 2.42.0
> >>
>
On Thu, Mar 28, 2024 at 08:52:01AM +0000, Kieran Bingham wrote:
> Quoting [email protected] (2024-03-28 00:57:34)
> > On 3/27/24 17:47, Conor Dooley wrote:
> > > On Wed, Mar 27, 2024 at 05:17:03PM -0600, [email protected] wrote:
> > >> From: Dave Stevenson <[email protected]>
> > >>
> > >> imx258.yaml doesn't include the vendor prefix of sony, so
> > >> rename to add it.
> > >> Update the id entry and MAINTAINERS to match.
> > >>
> > >> Signed-off-by: Dave Stevenson <[email protected]>
> > >> Acked-by: Conor Dooley <[email protected]>
> > >
> > > This is a v1 with my ack, something has gone awry here. It's also
> > > missing your signoff. Did you pick up someone else's series?
> >
> > Yes, this is a continuation of Dave's work. I contacted him directly,
> > and he mentioned that he is unable to submit a v2 any time soon and
> > was open to someone else continuing it in his stead.
Ah okay. Unfortunately I see so many binding patches pass by that I
sometimes forget about what I already reviewed, and I did not
remember this one at all.
> > This is my first
> > time submitting a patch via a mailing list, so I'm not sure if I'm
> > missing something, but I only added my sign off for anything that
> > actually included work from my side and not just bringing his patch
> > forward to this patch series.
Right. The rules are that you need to add it when you send someone's
work, like chain of custody type of thing.
> Your cover letter states v2, but the individual patches do not.
>
> Add the '-v2' (or, rather, next it will be '-v3') to git format-patch
> when you save your series and it will add the version to each patch. You
> can also add '-s' to that command I believe to add your SoB to each
> patch.
or a rebase will do it with --signoff:
https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---signoff
Cheers,
Conor.
On 3/28/24 04:15, Conor Dooley wrote:
> On Thu, Mar 28, 2024 at 08:52:01AM +0000, Kieran Bingham wrote:
>> Quoting [email protected] (2024-03-28 00:57:34)
>>> On 3/27/24 17:47, Conor Dooley wrote:
>>>> On Wed, Mar 27, 2024 at 05:17:03PM -0600, [email protected] wrote:
>>>>> From: Dave Stevenson <[email protected]>
>>>>>
>>>>> imx258.yaml doesn't include the vendor prefix of sony, so
>>>>> rename to add it.
>>>>> Update the id entry and MAINTAINERS to match.
>>>>>
>>>>> Signed-off-by: Dave Stevenson <[email protected]>
>>>>> Acked-by: Conor Dooley <[email protected]>
>>>>
>>>> This is a v1 with my ack, something has gone awry here. It's also
>>>> missing your signoff. Did you pick up someone else's series?
>>>
>>> Yes, this is a continuation of Dave's work. I contacted him directly,
>>> and he mentioned that he is unable to submit a v2 any time soon and
>>> was open to someone else continuing it in his stead.
>
> Ah okay. Unfortunately I see so many binding patches pass by that I
> sometimes forget about what I already reviewed, and I did not
> remember this one at all.
No worries I'm not surprised since i see constant things submitted
to upstream and v1 was actually sent a year ago so there would be no
shot that i would remember it either.
>
>>> This is my first
>>> time submitting a patch via a mailing list, so I'm not sure if I'm
>>> missing something, but I only added my sign off for anything that
>>> actually included work from my side and not just bringing his patch
>>> forward to this patch series.
>
> Right. The rules are that you need to add it when you send someone's
> work, like chain of custody type of thing.
Ohh i see, ok ill go ahead and add my sign off to all the patches then
>
>> Your cover letter states v2, but the individual patches do not.
>>
>> Add the '-v2' (or, rather, next it will be '-v3') to git format-patch
>> when you save your series and it will add the version to each patch. You
>> can also add '-s' to that command I believe to add your SoB to each
>> patch.
>
> or a rebase will do it with --signoff:
> https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---signoff
Perfect thanks for the information you two! Ill be sure to use those
for the next revision.
>
> Cheers,
> Conor.
On 28/03/2024 18:05, Luigi311 wrote:
>
> Looks like it no longer complains when i run
> make dt_binding_check DT_SCHEMA_FILES=media/i2c/sony,imx258
>
> with the following
>
> properties:
> compatible:
> enum:
> - sony,imx258
> - sony,imx258-pdaf
>
> If that looks good I can go ahead and include that in the v3
>
Looks good, thanks.
Best regards,
Krzysztof
On 3/28/24 01:47, Krzysztof Kozlowski wrote:
> On 28/03/2024 00:17, [email protected] wrote:
>> From: Dave Stevenson <[email protected]>
>>
>> There are a number of variants of the imx258 modules that can not
>> be differentiated at runtime, so add compatible strings for them.
>>
>> Signed-off-by: Dave Stevenson <[email protected]>
>> Signed-off-by: Luigi311 <[email protected]>
>> ---
>> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> index bee61a443b23..c7856de15ba3 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> @@ -14,10 +14,14 @@ description: |-
>> type stacked image sensor with a square pixel array of size 4208 x 3120. It
>> is programmable through I2C interface. Image data is sent through MIPI
>> CSI-2.
>> + There are a number of variants of the sensor which cannot be detected at
>> + runtime, so multiple compatible strings are required to differentiate these.
>>
>> properties:
>> compatible:
>> - const: sony,imx258
>> + - enum:
>> + - sony,imx258
>
> Two people working on patch but no one tested it before sending. Do not
> send untested code.
>
> It does not look like you tested the bindings, at least after quick
> look. Please run `make dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.
Hello, looks like I messed this up during my v2 (sorry missed the v in
my format patch) when I took this off Dave's hands. This is all new to
me so thank you for the command used to check, I was only compiling
the kernel and testing that so I didn't realize this needed separate
testing.
Looks like it no longer complains when i run
make dt_binding_check DT_SCHEMA_FILES=media/i2c/sony,imx258
with the following
properties:
compatible:
enum:
- sony,imx258
- sony,imx258-pdaf
If that looks good I can go ahead and include that in the v3
>
> Best regards,
> Krzysztof
>
On Wed, Mar 27, 2024 at 05:17:04PM -0600, [email protected] wrote:
> From: Dave Stevenson <[email protected]>
>
> There are a number of variants of the imx258 modules that can not
> be differentiated at runtime, so add compatible strings for them.
But you are only adding 1 variant.
>
> Signed-off-by: Dave Stevenson <[email protected]>
> Signed-off-by: Luigi311 <[email protected]>
> ---
> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> index bee61a443b23..c7856de15ba3 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> @@ -14,10 +14,14 @@ description: |-
> type stacked image sensor with a square pixel array of size 4208 x 3120. It
> is programmable through I2C interface. Image data is sent through MIPI
> CSI-2.
> + There are a number of variants of the sensor which cannot be detected at
> + runtime, so multiple compatible strings are required to differentiate these.
That's more reasoning/why for the patch than description of the h/w.
> properties:
> compatible:
> - const: sony,imx258
> + - enum:
> + - sony,imx258
> + - sony,imx258-pdaf
How do I know which one to use? Please define what PDAF means somewhere
as well as perhaps what the original/default variant is or isn't.
>
> assigned-clocks: true
> assigned-clock-parents: true
> --
> 2.42.0
>
On 3/28/24 12:55, Rob Herring wrote:
> On Wed, Mar 27, 2024 at 05:17:04PM -0600, [email protected] wrote:
>> From: Dave Stevenson <[email protected]>
>>
>> There are a number of variants of the imx258 modules that can not
>> be differentiated at runtime, so add compatible strings for them.
>> But you are only adding 1 variant.
I can not speak for Dave but as to why this was added here but looking
at the imx296 yaml that has something similar where there are multiple
variants that may not be detectable at run time but does not include
similar verbiage in the main description. Should I drop this from the
description so it matches the imx296?
>
>>
>> Signed-off-by: Dave Stevenson <[email protected]>
>> Signed-off-by: Luigi311 <[email protected]>
>> ---
>> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> index bee61a443b23..c7856de15ba3 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> @@ -14,10 +14,14 @@ description: |-
>> type stacked image sensor with a square pixel array of size 4208 x 3120. It
>> is programmable through I2C interface. Image data is sent through MIPI
>> CSI-2.
>> + There are a number of variants of the sensor which cannot be detected at
>> + runtime, so multiple compatible strings are required to differentiate these.
>
> That's more reasoning/why for the patch than description of the h/w.
>
>> properties:
>> compatible:
>> - const: sony,imx258
>> + - enum:
>> + - sony,imx258
>> + - sony,imx258-pdaf
>
> How do I know which one to use? Please define what PDAF means somewhere
> as well as perhaps what the original/default variant is or isn't.
Would it make sense to change the properties to include a description like so
properties:
compatible:
enum:
- sony,imx258
- sony,imx258-pdaf
description:
The IMX258 sensor exists in two different models, a standard variant
(IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
The camera module does not expose the model through registers, so the
exact model needs to be specified.
>
>>
>> assigned-clocks: true
>> assigned-clock-parents: true
>> --
>> 2.42.0
>>
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on media-tree/master]
[also build test WARNING on linuxtv-media-stage/master linus/master v6.9-rc1 next-20240328]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/git-luigi311-com/media-i2c-imx258-Remove-unused-defines/20240328-072629
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20240327231710.53188-19-git%40luigi311.com
patch subject: [PATCH 18/23] dt-bindings: media: imx258: Add alternate compatible strings
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240329/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
--
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: ignoring, error in schema: properties: compatible
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, Mar 28, 2024 at 01:16:22PM -0600, Luigi311 wrote:
> On 3/28/24 12:55, Rob Herring wrote:
> > On Wed, Mar 27, 2024 at 05:17:04PM -0600, [email protected] wrote:
> >> From: Dave Stevenson <[email protected]>
> >>
> >> There are a number of variants of the imx258 modules that can not
> >> be differentiated at runtime, so add compatible strings for them.
> >> But you are only adding 1 variant.
>
> I can not speak for Dave but as to why this was added here but looking
> at the imx296 yaml that has something similar where there are multiple
> variants that may not be detectable at run time but does not include
> similar verbiage in the main description. Should I drop this from the
> description so it matches the imx296?
Just change "add compatible strings for them" to "add compatible string
for the PDAF variant" or something.
>
> >
> >>
> >> Signed-off-by: Dave Stevenson <[email protected]>
> >> Signed-off-by: Luigi311 <[email protected]>
> >> ---
> >> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
> >> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> index bee61a443b23..c7856de15ba3 100644
> >> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> >> @@ -14,10 +14,14 @@ description: |-
> >> type stacked image sensor with a square pixel array of size 4208 x 3120. It
> >> is programmable through I2C interface. Image data is sent through MIPI
> >> CSI-2.
> >> + There are a number of variants of the sensor which cannot be detected at
> >> + runtime, so multiple compatible strings are required to differentiate these.
> >
> > That's more reasoning/why for the patch than description of the h/w.
> >
> >> properties:
> >> compatible:
> >> - const: sony,imx258
> >> + - enum:
> >> + - sony,imx258
> >> + - sony,imx258-pdaf
> >
> > How do I know which one to use? Please define what PDAF means somewhere
> > as well as perhaps what the original/default variant is or isn't.
>
> Would it make sense to change the properties to include a description like so
>
> properties:
> compatible:
> enum:
> - sony,imx258
> - sony,imx258-pdaf
> description:
> The IMX258 sensor exists in two different models, a standard variant
> (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
> The camera module does not expose the model through registers, so the
> exact model needs to be specified.
Looks fine, but I'd move this to the top-level 'description'.
Rob
On Wed, Mar 27, 2024 at 05:17:08PM -0600, [email protected] wrote:
> From: Luigi311 <[email protected]>
>
> On some boards powerdown signal needs to be deasserted for this
> sensor to be enabled.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> ---
> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++
Bindings should be a separate patch.
> drivers/media/i2c/imx258.c | 13 +++++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> index c7856de15ba3..0414085bf22f 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> @@ -35,6 +35,10 @@ properties:
> reg:
> maxItems: 1
>
> + powerdown-gpios:
> + description: |-
Don't need '|-' if no formatting.
> + Reference to the GPIO connected to the PWDN pin, if any.
> +
> reset-gpios:
> description: |-
> Reference to the GPIO connected to the XCLR pin, if any.
On 3/28/24 14:46, Rob Herring wrote:
> On Thu, Mar 28, 2024 at 01:16:22PM -0600, Luigi311 wrote:
>> On 3/28/24 12:55, Rob Herring wrote:
>>> On Wed, Mar 27, 2024 at 05:17:04PM -0600, [email protected] wrote:
>>>> From: Dave Stevenson <[email protected]>
>>>>
>>>> There are a number of variants of the imx258 modules that can not
>>>> be differentiated at runtime, so add compatible strings for them.
>>>> But you are only adding 1 variant.
>>
>> I can not speak for Dave but as to why this was added here but looking
>> at the imx296 yaml that has something similar where there are multiple
>> variants that may not be detectable at run time but does not include
>> similar verbiage in the main description. Should I drop this from the
>> description so it matches the imx296?
>
> Just change "add compatible strings for them" to "add compatible string
> for the PDAF variant" or something.
>
Ohh i see what you mean now, this is in reference to the commit message,
it was throwing me off because the imx258 description had almost the
exact same wording. Yes that makes sense, ill change the commit
message to specify PDAF.
>>
>>>
>>>>
>>>> Signed-off-by: Dave Stevenson <[email protected]>
>>>> Signed-off-by: Luigi311 <[email protected]>
>>>> ---
>>>> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>>>> index bee61a443b23..c7856de15ba3 100644
>>>> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>>>> @@ -14,10 +14,14 @@ description: |-
>>>> type stacked image sensor with a square pixel array of size 4208 x 3120. It
>>>> is programmable through I2C interface. Image data is sent through MIPI
>>>> CSI-2.
>>>> + There are a number of variants of the sensor which cannot be detected at
>>>> + runtime, so multiple compatible strings are required to differentiate these.
>>>
>>> That's more reasoning/why for the patch than description of the h/w.
>>>
>>>> properties:
>>>> compatible:
>>>> - const: sony,imx258
>>>> + - enum:
>>>> + - sony,imx258
>>>> + - sony,imx258-pdaf
>>>
>>> How do I know which one to use? Please define what PDAF means somewhere
>>> as well as perhaps what the original/default variant is or isn't.
>>
>> Would it make sense to change the properties to include a description like so
>>
>> properties:
>> compatible:
>> enum:
>> - sony,imx258
>> - sony,imx258-pdaf
>> description:
>> The IMX258 sensor exists in two different models, a standard variant
>> (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
>> The camera module does not expose the model through registers, so the
>> exact model needs to be specified.
>
> Looks fine, but I'd move this to the top-level 'description'.
>
> Rob
Perfect ill move this to the top level description and remove that small section
that Dave added
On 3/28/24 14:48, Rob Herring wrote:
> On Wed, Mar 27, 2024 at 05:17:08PM -0600, [email protected] wrote:
>> From: Luigi311 <[email protected]>
>>
>> On some boards powerdown signal needs to be deasserted for this
>> sensor to be enabled.
>>
>> Signed-off-by: Ondrej Jirman <[email protected]>
>> ---
>> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++
>
> Bindings should be a separate patch.
>
Ok ill create separate patch for adding in the binding and then
a follow up patch with the other half that actually adds it to
the driver
>> drivers/media/i2c/imx258.c | 13 +++++++++++++
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> index c7856de15ba3..0414085bf22f 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> @@ -35,6 +35,10 @@ properties:
>> reg:
>> maxItems: 1
>>
>> + powerdown-gpios:
>> + description: |-
>
> Don't need '|-' if no formatting>
Done
>> + Reference to the GPIO connected to the PWDN pin, if any.
>> +
>> reset-gpios:
>> description: |-
>> Reference to the GPIO connected to the XCLR pin, if any.
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on media-tree/master]
[also build test WARNING on linuxtv-media-stage/master linus/master v6.9-rc1 next-20240328]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/git-luigi311-com/media-i2c-imx258-Remove-unused-defines/20240328-072629
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20240327231710.53188-19-git%40luigi311.com
patch subject: [PATCH 18/23] dt-bindings: media: imx258: Add alternate compatible strings
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240330/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
--
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: ignoring, error in schema: properties: compatible
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wednesday, March 27, 2024 11:16:46 PM UTC [email protected] wrote:
> From: Luigi311 <[email protected]>
The Linux kernel does not allow anonymous (or pseudonymous) contributions. You
should use your real first and last name.
See section 1.5 in "A guide to the Kernel Development Process":
https://www.kernel.org/doc/html/v6.8/process/1.Intro.html#licensing
On 3/29/24 21:51, Dang Huynh wrote:
> On Wednesday, March 27, 2024 11:16:46 PM UTC [email protected] wrote:
>> From: Luigi311 <[email protected]>
>
> The Linux kernel does not allow anonymous (or pseudonymous) contributions. You
> should use your real first and last name.
>
> See section 1.5 in "A guide to the Kernel Development Process":
> https://www.kernel.org/doc/html/v6.8/process/1.Intro.html#licensing
>
>
Ok I've changed my sign off to my real first and last name for the
next revision
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on media-tree/master]
[also build test WARNING on linuxtv-media-stage/master linus/master v6.9-rc1 next-20240328]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/git-luigi311-com/media-i2c-imx258-Remove-unused-defines/20240328-072629
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20240327231710.53188-19-git%40luigi311.com
patch subject: [PATCH 18/23] dt-bindings: media: imx258: Add alternate compatible strings
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240331/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
--
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: ignoring, error in schema: properties: compatible
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/git-luigi311-com/media-i2c-imx258-Remove-unused-defines/20240328-072629
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20240327231710.53188-23-git%40luigi311.com
patch subject: [PATCH 22/23] drivers: media: i2c: imx258: Add support for powerdown gpio
config: x86_64-randconfig-161-20240331 (https://download.01.org/0day-ci/archive/20240401/[email protected]/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/
smatch warnings:
drivers/media/i2c/imx258.c:1562 imx258_probe() warn: missing unwind goto?
vim +1562 drivers/media/i2c/imx258.c
d3773094af21c9 Dave Stevenson 2024-03-27 1476
e4802cb00bfe3d Jason Chen 2018-05-02 1477 static int imx258_probe(struct i2c_client *client)
e4802cb00bfe3d Jason Chen 2018-05-02 1478 {
e4802cb00bfe3d Jason Chen 2018-05-02 1479 struct imx258 *imx258;
786d2ad50b9b49 Dave Stevenson 2024-03-27 1480 struct fwnode_handle *endpoint;
786d2ad50b9b49 Dave Stevenson 2024-03-27 1481 struct v4l2_fwnode_endpoint ep = {
786d2ad50b9b49 Dave Stevenson 2024-03-27 1482 .bus_type = V4L2_MBUS_CSI2_DPHY
786d2ad50b9b49 Dave Stevenson 2024-03-27 1483 };
e4802cb00bfe3d Jason Chen 2018-05-02 1484 int ret;
e4802cb00bfe3d Jason Chen 2018-05-02 1485 u32 val = 0;
e4802cb00bfe3d Jason Chen 2018-05-02 1486
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1487 imx258 = devm_kzalloc(&client->dev, sizeof(*imx258), GFP_KERNEL);
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1488 if (!imx258)
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1489 return -ENOMEM;
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1490
d3773094af21c9 Dave Stevenson 2024-03-27 1491 ret = imx258_get_regulators(imx258, client);
d3773094af21c9 Dave Stevenson 2024-03-27 1492 if (ret)
d3773094af21c9 Dave Stevenson 2024-03-27 1493 return dev_err_probe(&client->dev, ret,
d3773094af21c9 Dave Stevenson 2024-03-27 1494 "failed to get regulators\n");
d3773094af21c9 Dave Stevenson 2024-03-27 1495
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1496 imx258->clk = devm_clk_get_optional(&client->dev, NULL);
d170b0ea176098 Sakari Ailus 2021-08-16 1497 if (IS_ERR(imx258->clk))
d170b0ea176098 Sakari Ailus 2021-08-16 1498 return dev_err_probe(&client->dev, PTR_ERR(imx258->clk),
d170b0ea176098 Sakari Ailus 2021-08-16 1499 "error getting clock\n");
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1500 if (!imx258->clk) {
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1501 dev_dbg(&client->dev,
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1502 "no clock provided, using clock-frequency property\n");
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1503
e4802cb00bfe3d Jason Chen 2018-05-02 1504 device_property_read_u32(&client->dev, "clock-frequency", &val);
d170b0ea176098 Sakari Ailus 2021-08-16 1505 } else {
d170b0ea176098 Sakari Ailus 2021-08-16 1506 val = clk_get_rate(imx258->clk);
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1507 }
8bde18cb296d0e Dave Stevenson 2024-03-27 1508
8bde18cb296d0e Dave Stevenson 2024-03-27 1509 switch (val) {
8bde18cb296d0e Dave Stevenson 2024-03-27 1510 case 19200000:
8bde18cb296d0e Dave Stevenson 2024-03-27 1511 imx258->link_freq_configs = link_freq_configs_19_2;
8bde18cb296d0e Dave Stevenson 2024-03-27 1512 imx258->link_freq_menu_items = link_freq_menu_items_19_2;
8bde18cb296d0e Dave Stevenson 2024-03-27 1513 break;
8bde18cb296d0e Dave Stevenson 2024-03-27 1514 case 24000000:
8bde18cb296d0e Dave Stevenson 2024-03-27 1515 imx258->link_freq_configs = link_freq_configs_24;
8bde18cb296d0e Dave Stevenson 2024-03-27 1516 imx258->link_freq_menu_items = link_freq_menu_items_24;
8bde18cb296d0e Dave Stevenson 2024-03-27 1517 break;
8bde18cb296d0e Dave Stevenson 2024-03-27 1518 default:
8bde18cb296d0e Dave Stevenson 2024-03-27 1519 dev_err(&client->dev, "input clock frequency of %u not supported\n",
8bde18cb296d0e Dave Stevenson 2024-03-27 1520 val);
e4802cb00bfe3d Jason Chen 2018-05-02 1521 return -EINVAL;
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1522 }
e4802cb00bfe3d Jason Chen 2018-05-02 1523
786d2ad50b9b49 Dave Stevenson 2024-03-27 1524 endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
786d2ad50b9b49 Dave Stevenson 2024-03-27 1525 if (!endpoint) {
786d2ad50b9b49 Dave Stevenson 2024-03-27 1526 dev_err(&client->dev, "Endpoint node not found\n");
786d2ad50b9b49 Dave Stevenson 2024-03-27 1527 return -EINVAL;
786d2ad50b9b49 Dave Stevenson 2024-03-27 1528 }
786d2ad50b9b49 Dave Stevenson 2024-03-27 1529
786d2ad50b9b49 Dave Stevenson 2024-03-27 1530 ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep);
786d2ad50b9b49 Dave Stevenson 2024-03-27 1531 fwnode_handle_put(endpoint);
786d2ad50b9b49 Dave Stevenson 2024-03-27 1532 if (ret) {
786d2ad50b9b49 Dave Stevenson 2024-03-27 1533 dev_err(&client->dev, "Parsing endpoint node failed\n");
786d2ad50b9b49 Dave Stevenson 2024-03-27 1534 return ret;
786d2ad50b9b49 Dave Stevenson 2024-03-27 1535 }
786d2ad50b9b49 Dave Stevenson 2024-03-27 1536
786d2ad50b9b49 Dave Stevenson 2024-03-27 1537 /* Get number of data lanes */
a42d61a239fac8 Dave Stevenson 2024-03-27 1538 switch (ep.bus.mipi_csi2.num_data_lanes) {
a42d61a239fac8 Dave Stevenson 2024-03-27 1539 case 2:
a42d61a239fac8 Dave Stevenson 2024-03-27 1540 imx258->lane_mode_idx = IMX258_2_LANE_MODE;
a42d61a239fac8 Dave Stevenson 2024-03-27 1541 break;
a42d61a239fac8 Dave Stevenson 2024-03-27 1542 case 4:
a42d61a239fac8 Dave Stevenson 2024-03-27 1543 imx258->lane_mode_idx = IMX258_4_LANE_MODE;
a42d61a239fac8 Dave Stevenson 2024-03-27 1544 break;
a42d61a239fac8 Dave Stevenson 2024-03-27 1545 default:
786d2ad50b9b49 Dave Stevenson 2024-03-27 1546 dev_err(&client->dev, "Invalid data lanes: %u\n",
a42d61a239fac8 Dave Stevenson 2024-03-27 1547 ep.bus.mipi_csi2.num_data_lanes);
786d2ad50b9b49 Dave Stevenson 2024-03-27 1548 ret = -EINVAL;
786d2ad50b9b49 Dave Stevenson 2024-03-27 1549 goto error_endpoint_free;
786d2ad50b9b49 Dave Stevenson 2024-03-27 1550 }
786d2ad50b9b49 Dave Stevenson 2024-03-27 1551
7db096053387db Dave Stevenson 2024-03-27 1552 imx258->csi2_flags = ep.bus.mipi_csi2.flags;
7db096053387db Dave Stevenson 2024-03-27 1553
a8bb93eeccfa73 Dave Stevenson 2024-03-27 1554 imx258->variant_cfg = of_device_get_match_data(&client->dev);
a8bb93eeccfa73 Dave Stevenson 2024-03-27 1555 if (!imx258->variant_cfg)
a8bb93eeccfa73 Dave Stevenson 2024-03-27 1556 imx258->variant_cfg = &imx258_cfg;
a8bb93eeccfa73 Dave Stevenson 2024-03-27 1557
8a1906e91c0093 Luigi311 2024-03-27 1558 /* request optional power down pin */
8a1906e91c0093 Luigi311 2024-03-27 1559 imx258->powerdown_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
8a1906e91c0093 Luigi311 2024-03-27 1560 GPIOD_OUT_HIGH);
8a1906e91c0093 Luigi311 2024-03-27 1561 if (IS_ERR(imx258->powerdown_gpio))
8a1906e91c0093 Luigi311 2024-03-27 @1562 return PTR_ERR(imx258->powerdown_gpio);
ret = PTR_ERR(imx258->powerdown_gpio);
goto error_endpoint_free;
8a1906e91c0093 Luigi311 2024-03-27 1563
e4802cb00bfe3d Jason Chen 2018-05-02 1564 /* Initialize subdev */
e4802cb00bfe3d Jason Chen 2018-05-02 1565 v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
e4802cb00bfe3d Jason Chen 2018-05-02 1566
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1567 /* Will be powered off via pm_runtime_idle */
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1568 ret = imx258_power_on(&client->dev);
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1569 if (ret)
786d2ad50b9b49 Dave Stevenson 2024-03-27 1570 goto error_endpoint_free;
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1571
e4802cb00bfe3d Jason Chen 2018-05-02 1572 /* Check module identity */
e4802cb00bfe3d Jason Chen 2018-05-02 1573 ret = imx258_identify_module(imx258);
e4802cb00bfe3d Jason Chen 2018-05-02 1574 if (ret)
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1575 goto error_identify;
e4802cb00bfe3d Jason Chen 2018-05-02 1576
e4802cb00bfe3d Jason Chen 2018-05-02 1577 /* Set default mode to max resolution */
e4802cb00bfe3d Jason Chen 2018-05-02 1578 imx258->cur_mode = &supported_modes[0];
e4802cb00bfe3d Jason Chen 2018-05-02 1579
e4802cb00bfe3d Jason Chen 2018-05-02 1580 ret = imx258_init_controls(imx258);
e4802cb00bfe3d Jason Chen 2018-05-02 1581 if (ret)
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1582 goto error_identify;
e4802cb00bfe3d Jason Chen 2018-05-02 1583
e4802cb00bfe3d Jason Chen 2018-05-02 1584 /* Initialize subdev */
e4802cb00bfe3d Jason Chen 2018-05-02 1585 imx258->sd.internal_ops = &imx258_internal_ops;
e4802cb00bfe3d Jason Chen 2018-05-02 1586 imx258->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
e4802cb00bfe3d Jason Chen 2018-05-02 1587 imx258->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
e4802cb00bfe3d Jason Chen 2018-05-02 1588
e4802cb00bfe3d Jason Chen 2018-05-02 1589 /* Initialize source pad */
e4802cb00bfe3d Jason Chen 2018-05-02 1590 imx258->pad.flags = MEDIA_PAD_FL_SOURCE;
e4802cb00bfe3d Jason Chen 2018-05-02 1591
e4802cb00bfe3d Jason Chen 2018-05-02 1592 ret = media_entity_pads_init(&imx258->sd.entity, 1, &imx258->pad);
e4802cb00bfe3d Jason Chen 2018-05-02 1593 if (ret)
e4802cb00bfe3d Jason Chen 2018-05-02 1594 goto error_handler_free;
e4802cb00bfe3d Jason Chen 2018-05-02 1595
15786f7b564eff Sakari Ailus 2021-03-05 1596 ret = v4l2_async_register_subdev_sensor(&imx258->sd);
e4802cb00bfe3d Jason Chen 2018-05-02 1597 if (ret < 0)
e4802cb00bfe3d Jason Chen 2018-05-02 1598 goto error_media_entity;
e4802cb00bfe3d Jason Chen 2018-05-02 1599
e4802cb00bfe3d Jason Chen 2018-05-02 1600 pm_runtime_set_active(&client->dev);
e4802cb00bfe3d Jason Chen 2018-05-02 1601 pm_runtime_enable(&client->dev);
e4802cb00bfe3d Jason Chen 2018-05-02 1602 pm_runtime_idle(&client->dev);
786d2ad50b9b49 Dave Stevenson 2024-03-27 1603 v4l2_fwnode_endpoint_free(&ep);
e4802cb00bfe3d Jason Chen 2018-05-02 1604
e4802cb00bfe3d Jason Chen 2018-05-02 1605 return 0;
e4802cb00bfe3d Jason Chen 2018-05-02 1606
e4802cb00bfe3d Jason Chen 2018-05-02 1607 error_media_entity:
e4802cb00bfe3d Jason Chen 2018-05-02 1608 media_entity_cleanup(&imx258->sd.entity);
e4802cb00bfe3d Jason Chen 2018-05-02 1609
e4802cb00bfe3d Jason Chen 2018-05-02 1610 error_handler_free:
e4802cb00bfe3d Jason Chen 2018-05-02 1611 imx258_free_controls(imx258);
e4802cb00bfe3d Jason Chen 2018-05-02 1612
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1613 error_identify:
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1614 imx258_power_off(&client->dev);
9fda25332c4b9e Krzysztof Kozlowski 2021-01-27 1615
786d2ad50b9b49 Dave Stevenson 2024-03-27 1616 error_endpoint_free:
786d2ad50b9b49 Dave Stevenson 2024-03-27 1617 v4l2_fwnode_endpoint_free(&ep);
786d2ad50b9b49 Dave Stevenson 2024-03-27 1618
e4802cb00bfe3d Jason Chen 2018-05-02 1619 return ret;
e4802cb00bfe3d Jason Chen 2018-05-02 1620 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki