[PATCH v2] arm64: dts: rk3399-pinephone-pro: Add internal display support
Javier Martinez Canillas
javierm at redhat.com
Mon Mar 27 20:08:21 PDT 2023
Ondřej Jirman <megi at xff.cz> writes:
> On Mon, Mar 27, 2023 at 08:15:52PM +0200, Javier Martinez Canillas wrote:
>> Ondřej Jirman <megi at xff.cz> writes:
>> > Just because something is in my tree doesn't mean it's mine, or that I reviewed
>> > it in detail and prepared it for upstreaming, or that I'm interested in
>> Thanks for the clarification. Because the patch had your authorship I
>> wrongly assumed that came from you. Sorry about the confusion.
> Ever since base DT support for Pinephone Pro was merged, none of the DT patches
> in my tree are in the original form as prepared by the authors + fixes I've
> added. That's simply impossible anymore.
> To look up who did what, you'd have to look at older branches, pre-merge.
> Patches after the merge just came from squashing everything into one patch,
> cleaning it up, and re-splitting along some vague functionality boundaries,
> while trying to keep best-effort original SoBs as faithfully as possible, so
> that I can keep maintaining the PPP support in a sane manner.
> Anyway, SoB's are added in chronological order. So:
> Means the author of the changes is Martijn + Kamil (roughly) and I may have
> a line of code in there too, since my SoB is last. For some reason, the order is
> inverted in the patch you posted, making it seem I developed these changes
Since the patch had your authorship I changed the order so that your S-o-B
was first but I'll then change the author in v3 and make it match the
first S-o-B entry in your tree (Martijn) then.
>> > upstreaming it. I'm just trying to help you with your upstreaming effort by
>> > testing and review since I got to know the hardware quite well over the last
>> > years and can check the schematics and datasheets quickly, and I like to think
>> > upstream code is held to higher standard. That's all.
>> Appreciate your help and I agree that upstream code should be held to a
>> high standard. But since the DTS in mainline is pretty basic anyways (you
>> can only boot to serial right now), is not really usable for other thing
>> than development and keep adding the missing support.
> Having wrong frequency used is not a missing support for something. Sorry it's
> too bothersome to get the review piecemeal, but sometimes people have more time to
> look at patches in-depth, and at other times they don't and you just get surface
> level or no review.
> One point of posting patches to the mailing list is to get review. And it's not
> that great to do in-depth review for you, up to going through schematics and
> datasheets, testing, and even proposing and testing solutions for found issues,
> just to be dismissed without technical reason.
> The thing is this review will most likely happen just once, and noone will go
> back, read through the entire huge DT along with a schematic, to look at whether
> this or that pullup is really necessary, whether some parameter is out of spec
> from the datasheet for each part, or things like that. That's just not
> Instead, people will happily attribute non-obvious issues caused by these
> omissions of manual review to shoddy or slow or power-inefficient HW. "1kHz
> + harmonics interference in codec because high power backlight DC-DC regulator
> basically spews off 1kHz of 1-2W load + harmonics because it's driven
> incorrectly? Ah, the phone just has a shitty audio codec!"
> So, don't take it as some perfectionism. Upstreaming just seems like the best
> time to look at things that were overlooked in the past in more detail and get
> these little things right, because the DT additions are done piecemal and
> slowly/gradually, so it's more manageable to review and fix right away. This
> will just not happen later on for these obscure devices like Pinephone Pro,
> where the whole effort that goes into it is like one half of a fulltime
> developer time split over 4 mildly interested real persons, slowly tapering off
> over time as the device ages.
Makes sense. I'll address your comments in v3 then and also include a
separate patch (again with Martijn as author and the S-o-B as ordered in
your tree), that includes the touchscreen DT nodes as well. Since Jarrah
pointed out that there's already the correct compatible string in the
driver's OF device ID table.
Javier Martinez Canillas
More information about the Linux-rockchip