On Wed, 12 Aug 2020, Mauro Carvalho Chehab wrote: > Hi Greg, > > This patch series is part of a work I'm doing in order to be able to support > a HiKey 970 board that I recently got on my hands. > > I received some freedback from Mark and from Jonathan on a first > attempt I made to upstream this. > > I'm opting to submit it via staging, because I had to start from the > patch that originally added this driver on a 4.9 Kernel tree: > > https://github.com/96boards-hikey/linux/tree/hikey970-v4.9 > > In order to preserve the original SOB from the driver's author. > > The patches following it are on the standard way: one patch per > logical change. > > This is part of a bigger work whose goal is to have upstream support > for USB and DRM/KMS on such boards. > > I suspect that, maybe except for the DT part, those 3 specific drivers > are more or less ready to be moved from staging, but the other > drivers that are also part of this attempt aren't ready. Specially the > DRM driver has some bugs that came from the OOT version. > > So, my current plan is to submit those drivers to staging for 5.9 > and move the ones that are ok out of staging on Kernel 5.10. What a mess. This is no way to upstream a new driver. Firstly, could you please add versioning to your submissions. I know this at least version 2. Were there previous submissions? Is this the latest? Secondly and more importantly, you have submitted what looks like a new driver (bearing in mind that I'm only concerning myself with the MFD related changes), then in the same submission you are adding and removing large chunks. Please just submit the new driver, on its own as a single patch, complete with its associated Makefile and Kconfig changes. What are your reasons for submitting this via Staging? Is it not ready yet? Are the resultant components not at a high enough level of quality or enablement to go straight into the subsystems, which is more typical? From an MFD perspective, I would be reviewing the driver as a whole when (if) it moves from Staging into MFD anyway, so why are you jumping through hoops with this additional, seemingly superfluous step? Finally, the subject of authorship is often a contentious one, but this is a problem you need to work out with the original author, not something that should require special handing by upstream. You have a couple of choices, but bear in mind that upstreaming a non-suitable driver then bringing it up to standard is not one of them. 1. Keep the original author's authorship and SoB, make your changes and get them to review to ensure they are still happy about being associated with the resultant code. Ensure you mention all of the changes you make in the commit message and follow-up by adding your own SoB. 2. This is the contentious bit. If you've made enough changes, there is an argument for you to adopt authorship. You should discuss with the original author whether they are happy for you to retain their SoB. My suggestion is always try to keep the SoB as a bare minimum to preserve patch history and out of pure courtesy. > Mauro Carvalho Chehab (41): > staging: spmi: hisi-spmi-controller: coding style fixup > staging: spmi: hisi-spmi-controller: fix it to probe successfully > staging: spmi: hisi-spmi-controller: fix a typo > staging: spmi: hisi-spmi-controller: adjust whitespaces at defines > staging: spmi: hisi-spmi-controller: use le32 macros where needed > staging: spmi: hisi-spmi-controller: add debug when values are > read/write > staging: spmi: hisi-spmi-controller: fix the dev_foo() logic > staging: spmi: hisi-spmi-controller: add it to the building system > staging: spmi: hisi-spmi-controller: do some code cleanups > staging: mfd: hi6421-spmi-pmic: get rid of unused code > staging: mfd: hi6421-spmi-pmic: deal with non-static functions > staging: mfd: hi6421-spmi-pmic: get rid of the static vars > staging: mfd: hi6421-spmi-pmic: cleanup hi6421-spmi-pmic.h header > staging: mfd: hi6421-spmi-pmic: change the binding logic > staging: mfd: hi6421-spmi-pmic: get rid of unused OF properties > staging: mfd: hi6421-spmi-pmic: cleanup OF properties > staging: mfd: hi6421-spmi-pmic: change namespace on its functions > staging: mfd: hi6421-spmi-pmic: fix some coding style issues > staging: mfd: hi6421-spmi-pmic: add it to the building system > staging: mfd: hi6421-spmi-pmic: cleanup the code > staging: regulator: hi6421v600-regulator: get rid of unused code > staging: regulator: hi6421v600-regulator: port it to upstream > staging: regulator: hi6421v600-regulator: coding style fixups > staging: regulator: hi6421v600-regulator: change the binding logic > staging: regulator: hi6421v600-regulator: cleanup struct > hisi_regulator > staging: regulator: hi6421v600-regulator: cleanup debug messages > staging: regulator: hi6421v600-regulator: use shorter names for OF > properties > staging: regulator: hi6421v600-regulator: better handle modes > staging: regulator: hi6421v600-regulator: change namespace > staging: regulator: hi6421v600-regulator: convert to use get/set > voltage_sel > staging: regulator: hi6421v600-regulator: don't use usleep_range for > off_on_delay > staging: regulator: hi6421v600-regulator: add a driver-specific debug > macro > staging: regulator: hi6421v600-regulator: initialize ramp_delay > staging: regulator: hi6421v600-regulator: cleanup DT settings > staging: regulator: hi6421v600-regulator: fix some coding style issues > staging: regulator: hi6421v600-regulator: add it to the building > system > staging: regulator: hi6421v600-regulator: code cleanup > staging: hikey9xx: add a TODO list > MAINTAINERS: add an entry for HiSilicon 6421v600 drivers > dt: document HiSilicon SPMI controller and mfd/regulator properties > dt: hisilicon: add support for the PMIC found on Hikey 970 > > Mayulong (3): > staging: spmi: add Hikey 970 SPMI controller driver > staging: mfd: add a PMIC driver for HiSilicon 6421 SPMI version > staging: regulator: add a regulator driver for HiSilicon 6421v600 SPMI > PMIC > > .../mfd/hisilicon,hi6421-spmi-pmic.yaml | 182 +++++++ > .../spmi/hisilicon,hisi-spmi-controller.yaml | 54 ++ > MAINTAINERS | 6 + > .../boot/dts/hisilicon/hi3670-hikey970.dts | 16 +- > .../boot/dts/hisilicon/hikey970-pmic.dtsi | 200 ++++++++ > drivers/staging/Kconfig | 2 + > drivers/staging/Makefile | 1 + > drivers/staging/hikey9xx/Kconfig | 35 ++ > drivers/staging/hikey9xx/Makefile | 5 + > drivers/staging/hikey9xx/TODO | 5 + > drivers/staging/hikey9xx/hi6421-spmi-pmic.c | 381 ++++++++++++++ > .../staging/hikey9xx/hi6421v600-regulator.c | 479 ++++++++++++++++++ > .../staging/hikey9xx/hisi-spmi-controller.c | 351 +++++++++++++ > include/linux/mfd/hi6421-spmi-pmic.h | 68 +++ > 14 files changed, 1773 insertions(+), 12 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml > create mode 100644 Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml > create mode 100644 arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi > create mode 100644 drivers/staging/hikey9xx/Kconfig > create mode 100644 drivers/staging/hikey9xx/Makefile > create mode 100644 drivers/staging/hikey9xx/TODO > create mode 100644 drivers/staging/hikey9xx/hi6421-spmi-pmic.c > create mode 100644 drivers/staging/hikey9xx/hi6421v600-regulator.c > create mode 100644 drivers/staging/hikey9xx/hisi-spmi-controller.c > create mode 100644 include/linux/mfd/hi6421-spmi-pmic.h > -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
