On 03-Feb-23 21:03, Tomi Valkeinen wrote: > On 25/01/2023 13:35, Aradhya Bhatia wrote: >> Add support for the DSS controller on TI's new AM625 SoC in the tidss >> driver. >> >> The first video port (VP0) in am625-dss can output OLDI signals through >> 2 OLDI TXes. A 3rd output port has been added with "DISPC_PORT_OLDI" bus >> type. > > Not a big thing here as you add support for a new SoC, but the ordering > of the patches is not optimal. Here you add the AM625 DSS support, but > then you continue actually adding the DSS support (well, mainly OLDI) in > the following patches. > > I think patch 6 could be before this patch. Parts of patch 4 could also > be before this patch. The AM65X renames from patch 5 could be before > this patch. I can move whole of Patch 6 and even of Patch 4 before this one. I have mentioned 'AM625-DSS' in a couple comments which I can make generic, and the rest everything is SoC-agnostic. I haven't tried this, but my concern is if we break patch 5 into 2 separate patches, i. AM65X rename plus SoC based switch case, and ii. Addition of AM625 SoC case then I might have to overwrite some changes implemented during (i) in (ii). I don't suppose that would be okay, would it? Also, is it important to keep the compatible-addition patches of DT-binding and driver next to each other in the series? Or should the DT-binding patches should be the first ones? Just curious! =) > > I'm mainly thinking of a case where someone uses AM625 and is bisecting > a problem. What happens if his board uses OLDI, and he happens to hit > one of these patches during bisect? If the display just stays black, but > otherwise everything works fine, then no problem. But if it crashes or > starts spamming sync losts or such or gives errors, it's not so nice. > You are right! This certainly makes sense. Regards Aradhya