On Sun, 15 May 2022 17:57:14 +0100 Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On Sun, 15 May 2022 17:30:04 +0200 > Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> wrote: > > > On 2022-05-14 17:13:12, Jonathan Cameron wrote: > > > On Thu, 12 May 2022 00:06:10 +0200 > > > Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> wrote: > > > > > > > These channels are specified in downstream kernels [1] and actively used > > > > by ie. the Sony Seine platform on the SM6125 SoC. > > > > > > Looking at the links, some of them are on that platform but not all. > > > Better to make that explicit in this description. > > > > This has already been queued up for v2. Adding these seemed easy at the > > time but they are in fact not used, and I ended up sending the wrong > > patch. > > > > Just so that we're on the same page: only ADC5_AMUX_THM3 and > > ADC5_GPIO2_100K_PU are unused by my platform. It seems the first should > > be dropped, but the latter can probably stay in the patch with an > > explicit mention. If you think both should stay, there are a bunch more > > channels defined in the downstream kernel as per [1] and I'm not sure if > > all should be added for completeness. > > Probably better to add them with a comment for platforms on which they > apply (either in commit log or alongside the definitions in the code). By 'them' I mean add the ones used on your platform only. Let others add theirs when / if boards using them are upstreamed. (realised I'd been very unclear just after hitting send!) > > Longer term we should think about whether the code can be adjusted > to not need an explicit definition for these multi purpose channels > though letting the dt itself describe them (under constraints of the > hardware platform). Not worth doing before this patch though. > > > > > > > > > > > [1]: https://source.codeaurora.org/quic/la/kernel/msm-4.14/tree/drivers/iio/adc/qcom-spmi-adc5.c?h=LA.UM.7.11.r1-05200-NICOBAR.0#n688 > > > > > > > > Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> > > > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx> > > > > > > I'm not keen on patches with no context being > > > sent to mailing lists. Please cc all lists (and preferably individuals) > > > on at least the cover letter so we can see overall discussion. > > > > That can be attributed to the terrible workflow for sending > > patch-series. Somehow only `git send-email` supports --cc-cmd yet I'd > > expect it on `git format-patch` for auditing and possibly copying to the > > cover letter, if `git format-patch --cover-letter` couldn't do this from > > the beginning. > > It would definitely be nice as an option. Can't do it every time because > on tree wide change the cc list can become so large the mailing lists > reject it. > > > At the same time `git send-email` has --[to/cc]-cover options to > > propagate email addresses from the cover letter to all the individual > > patch-replies, but not the inverse :( > > > > In the end this leaves me manually running get_maintainer.pl over the > > entire formatted patch-series, and manually copy-pasting + editing the > > addresses into the cover letter... Which is easy to forget and is no > > different here. > > > > My apologies for (yet again) accidentally not sending at least the cover > > letter to everyone. That's a gross oversight, and I'm probably - no, I > > must - be doing something wrong. Suggestions and/or documentation > > references are welcome. > > Andy Shevchenko has some scripts to try and help with this: > https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh > > I've not started using them myself (not gotten around to it yet!) but > he's pointed to them when I've missed people from cover letter cc lists > in the past. > > > > > > If nothing else, I've no idea if intent is that the patches go through different > > > trees or all need to merge via one route. > > > > I have no idea either, and have not yet had an answer to a similar > > question on a different list. Usually it seems the maintainers work out > > amongst themselves who picks what patch, putting them on hold where > > necessary to preseve ordering. If not, should the sender split patches > > across multiple series, either holding off sending part of it or linking > > to a dependent series? > > In this case I think I can pick this patch up directly into the IIO tree > once everyone else is happy. As you note dts patches normally wait on > knowing the necessary support is heading in. If you have a view on what > makes sense as the submitter it's good to stick it in the cover letter, but > in this case sounds like you don't. :) > > Given we are near the end of this cycle, we are probably looking at next > cycle anyway now, so plenty of time to figure it out! > > > > > In this particular case DT has to wait for these driver patches to land, > > otherwise they may define channels that do not exist and unnecessarily > > fail probe. > > > > > Patch itself looks fine, > > > > Thanks. > > > > Looking forward to your suggestions and answers, > > > > - Marijn > > > > > [..] >
