On Sun, Mar 3, 2019 at 3:52 PM Renato Lui Geh <renatogeh@xxxxxxxxx> wrote: > > Hi Alexandru, > > Thanks for the review. Some questions inline. > > Thanks, > Renato > > On 03/01, Ardelean, Alexandru wrote: > >On Thu, 2019-02-28 at 11:23 -0300, Renato Lui Geh wrote: > >> > > > >The patch-series is a bit big. > >I guess that the intent is to move this out-of-staging, but various patches > >are holding this in it's place. > >For patch series above a certain size, you could get many re-spins > >[V2,3,4... so on]. > > > >You could send some of the changes as individual patches, or group them in > >series of 1,2 or 3 patches. That way, you "parallelize" patch sending, and > >when you get reviews on each patch, you can re-spin them individually. > >You'll find over time that certain patches get accepted on V1, others on V2 > >and some on V7 [ hopefully, there isn't any frustration at that point ]. > > On these subseries, should versioning follow this patchset (v5) or should > they start anew (v1), ignoring this series version? I guess, in this case it's fine to leave it as is [in this series]. The series has been reviewed now. But [for me typically], I delay doing a review if a patch-series is longer than 4-5 patches. And I think some reviewers may do the same. So, if I want more people to review/look at my code, I try to make things as easy to review, as possible. And one way, is to definitely keep things decoupled. If one patch can be independent of another [for the same driver/code], I send them as separate patches. This [of course], is a preference. Some reviewers don't mind longer series [than 4-5 patches]. > > > >Well, this is a technique I use to distribute some of my upstream-patch- > >work, so that I can switch easier between internal-work & upstreaming-work. > > > >Coming back to this patch-series. > >My general input, is that the patches are fine over-all; some are just > >cosmetics/noise/a-different-way-of-doing-things-for-this-driver, and those > >usually can be left to preference [of the maintainer usually]. > > > >I do suggest to not hurry when re-spinning patches, and not change too much > >the number of patches in a new series. That can complicate things > >sometimes. But, if doing small patch-series or individual patches, you > >won't have this problem too much. > > > >Thanks > >Alex > > > >> > >> This series of patches contains the following: > >> - Adds user input for the 'gain' and 'filter' GPIO pins for the ad778x > >> family chips; > >> - Filter reading for the ad778x; > >> - Sets pattern macro values and mask for PATTERN status bits; > >> - Adds ID values for the ad7170, ad7171, ad7780 and ad7781 for ID > >> status bits checking; > >> - Moves regulator initialization to after GPIO init to maintain > >> consistency between probe and remove; > >> - Copyright edits, adding SPDX identifier and new copyright holder; > >> - Moves the ad7780 driver out of staging to the mainline; > >> - Adds device tree binding for the ad7780 driver. > >> > >> Renato Lui Geh (9): > >> staging: iio: ad7780: add gain & filter gpio support > >> staging: iio: ad7780: add filter reading to ad778x > >> staging: iio: ad7780: set pattern values and masks directly > >> staging:iio:ad7780: add chip ID values and mask > >> staging: iio: ad7780: move regulator to after GPIO init > >> staging: iio: ad7780: add SPDX identifier > >> staging: iio: ad7780: add new copyright holder > >> staging: iio: ad7780: moving ad7780 out of staging > >> staging: iio: ad7780: add device tree binding > >> > >> Changelog: > >> *v3 > >> - SPDX and regulator init as patches > >> - Renamed filter to odr and ad778x_filter to ad778x_odr_avail > >> - Removed unnecessary regulator disabling > >> - Removed unnecessary AD_SD_CHANNEL macro > >> - Changed unsigned int to unsigned long long to avoid overflow > >> *v4 > >> - Split gain & filter patch into two, with the new commit adding only > >> filter reading > >> - Changed pattern values to direct values, and added pattern mask > >> - Added ID values and mask > >> - Added new copyright holder > >> - Added device tree binding to the ad7780 driver > >> > >> .../bindings/iio/adc/adi,ad7780.txt | 48 +++ > >> drivers/iio/adc/Kconfig | 12 + > >> drivers/iio/adc/Makefile | 1 + > >> drivers/iio/adc/ad7780.c | 365 ++++++++++++++++++ > >> drivers/staging/iio/adc/Kconfig | 13 - > >> drivers/staging/iio/adc/Makefile | 1 - > >> drivers/staging/iio/adc/ad7780.c | 277 ------------- > >> 7 files changed, 426 insertions(+), 291 deletions(-) > >> create mode 100644 > >> Documentation/devicetree/bindings/iio/adc/adi,ad7780.txt > >> create mode 100644 drivers/iio/adc/ad7780.c > >> delete mode 100644 drivers/staging/iio/adc/ad7780.c > >> > >> -- > >> 2.21.0 > >> > > > >-- > >You received this message because you are subscribed to the Google Groups "Kernel USP" group. > >To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@xxxxxxxxxxxxxxxx. > >To post to this group, send email to kernel-usp@xxxxxxxxxxxxxxxx. > >To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/72a54cd5f58aeb9507b95b7e33ca3d9a38c853e9.camel%40analog.com. > >For more options, visit https://groups.google.com/d/optout.
