Hejssan, On Tue, Jan 07, 2025 at 10:52:19AM +0100, Niklas Söderlund wrote: > Hi Sakari, > > Tack för din feedback. > > On 2025-01-07 08:36:31 +0000, Sakari Ailus wrote: > > Hejssan Niklas, > > > > Tack för dessa lappar! > > > > On Sat, Jan 04, 2025 at 08:55:48PM +0100, Niklas Söderlund wrote: > > > Provided a safe-guard from the raw values used in device tree sources > > > and the in-kernel defines used to describe the different line orders. > > > This mimics what have been done for the bus-type property to provide the > > > same safe-guard. > > > > > > The macros used in device tree sources are defined in video-interfaces.h > > > (MEDIA_BUS_CSI2_CPHY_LINE_ORDER_*) and are only visible to DTS source > > > files. These raw values map directly to the in-kernel names by fwnode > > > defines in v4l2-fwnode.h (V4L2_FWNODE_CSI2_CPHY_LINE_ORDER_*). These > > > fwnode defines are finally translated to defines which are exposed to > > > drivers to act on (V4L2_MBUS_CSI2_CPHY_LINE_ORDER_*). > > > > > > Previously the translation to values provided to drivers have exploited > > > the fact that the numerical value for each setting are the same for the > > > defines used in device tree sources. While this is unlikely to change > > > this harmonises the bus-type and line-orders parsing to work using the > > > same mechanics, while at the same time make the large CSI-2 parsing > > > function a little more readable. > > > > Do we in fact need the V4L2_MBUS_ definitions of the line orders at all? > > I'm not sure :-) > > Geert pointed out in [1] that in comparison to the V4L2_MBUS_ bus-type > definitions the line-order definitions did not have this intermediary > step as a safe guard between values used in DTS files and values used in > V4L2 drivers. > > Looking at the original functionality, > > bus->line_orders[i] = array[i]; > > Seems a bit "hack" compared to what this patch do, > > > bus->line_orders[i] = v4l2_fwnode_line_order_to_mbus(array[i]); > > But if it's worth the extra churn, and if it in reality provides us with > a safe-guard between DTS-files and V4L2-drivers I'm not sure. I'm on the > fence on this one, the one good thing is that it aligns how V4L2_MBUS_ > macros are parsed. > > But if you don't like it and I'm on the fence I'm happy to drop this > series. This series don't add any extra functionality. I wasn't asking dropping the series, but instead get rid of the V4L2_MBUS_ line order definitions altogether, by replacing them by V4L2_FWNODE_ equivalents. > > 1. CAMuHMdXwqb7vhUeoMKDDJO5dp-V3LmnURZLSC1_ko=YL=cNyUA@xxxxxxxxxxxxxx > > > > > The same could extend to the V4L2_MBUS_ bus type defitions, but that's out > > of scope of this patch. > > Out of scope indeed. If we drop this series do we want to try and remove > them for V4L2_MBUS_ bus-type in future? I think that would be reasonable. I don't think we need two sets of definitions that effectively are interchangeable. But that may well be out of scope of this series. -- Med vänliga hälsningar, Sakari Ailus
