Re: [PATCH 00/44] SPMI patches needed by Hikey 970

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel