On 05/02/2023 16:31, Aradhya Bhatia wrote:
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?
I'm not sure I follow here. Wouldn't (i) be a valid patch in its own?
Nothing wrong in expanding that later (even if you end up changing a lot
of it).
That said, I don't think this is a very important topic. There are only
a few commits in the history that might be problematic. A simple fix
would be to add all the features first, and only last add the compatible
string for am625.
Or do all the changes for am625 in a single patch, and try to implement
all the generic restructuring work before that.
Here we do have to change the vp-to-output mapping management, so maybe
the second option won't be simple enough, and it's better to do the
am625 changes in pieces, as in the first option.
So, it's really up to you. Just wanted to raise this possible issue so
that you are aware of it and can do any easy fixes (if there are such).
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 believe the convention is to have the DT-binding changes before you
add the compatible string to the driver (if I recall right checkpatch or
some other checking tool complains if you add a driver for a compatible
that doesn't have a DT binding). Generic restructurings could be before
the DT patch, of course, but usually I like to keep the DT binding
changes at the very beginning of the series.
Tomi