Quoting Luis Garcia (2024-04-06 06:25:41) > On 4/3/24 12:45, Pavel Machek wrote: > > Hi! > > > >> +/* > >> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes. > >> + * To avoid further computation of clock settings, adopt the same per > >> + * 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 }, > >> +}; > >> + > >> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = { > >> { 0x0136, 0x13 }, > >> { 0x0137, 0x33 }, > >> { 0x0301, 0x05 }, > > > > I wish we did not have to copy all the magic values like this. > > > > Best regards, > > Pavel > > > > no kidding, magic values everywhere.... it makes it annoying > for me to move things around because they all start to look > similar. Down the line we added in more defined names so its > not as bad but still its bad lol. This series converts the defines to names, which is great. It would have been nicer if the series converted first, but I know the history here means you have done the register naming on top of existing patches - so I don't think there's a requirement to change the ordering now. But I see new drivers coming in with register tables. I hope we can start to apply more pressure to driver submitters to use higher quality named register sets in the future, now that we have a greater precendent of sensor drivers 'doing the right thing'. Sets of tables like we have are basically a binary blob stored as ascii and make maintainance far more difficult IMO. Maybe I should hit send on my comments on the latest GalaxyCore driver coming in that I hesitated on ... -- Kieran