Ondřej Jirman <megi@xxxxxx> writes: > On Mon, Mar 27, 2023 at 08:15:52PM +0200, Javier Martinez Canillas wrote: >> Ondřej Jirman <megi@xxxxxx> 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. > Go it. > Anyway, SoB's are added in chronological order. So: > > https://github.com/megous/linux/commit/471c5f33ba74c3d498ccc1eb69c098623b193926 > > 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 > originally. > 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. > Ok. > 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 > pragmatic. > That's fair. > 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. > regards, > o. > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
