Message-ID: <20171023091823.luvu5a4uxsw37bso@dell>
Date:   Mon, 23 Oct 2017 10:18:23 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Mario.Limonciello@...l.com
Cc:     rui_feng@...lsil.com.cn, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
        ricky_wu@...ltek.com, wei_wang@...lsil.com.cn
Subject: Re: staging: rtsx: Add support for RTS5260

On Mon, 16 Oct 2017, Mario.Limonciello@...l.com wrote:

> > -----Original Message-----
> > From: Lee Jones [mailto:lee.jones@...aro.org]
> > Sent: Friday, October 13, 2017 4:15 PM
> > To: Limonciello, Mario <Mario_Limonciello@...l.com>
> > Cc: rui_feng@...lsil.com.cn; gregkh@...uxfoundation.org; linux-
> > kernel@...r.kernel.org; devel@...verdev.osuosl.org; ricky_wu@...ltek.com;
> > wei_wang@...lsil.com.cn
> > Subject: Re: staging: rtsx: Add support for RTS5260
> > 
> > On Fri, 13 Oct 2017, Mario Limonciello wrote:
> > > On 10/13/2017 03:50 AM, rui_feng@...lsil.com.cn wrote:
> > > > From: rui_feng <rui_feng@...lsil.com.cn>
> > > >
> > > > Add support for new chip rts5260.
> > > > In order to support rts5260，the definitions of some internal
> > > > registers and workflow have to be modified and are different from its
> > > > predecessors and OCP function is added for RTS5260.
> > > > So we need this patch to ensure RTS5260 can work.
> > > >
> > > > Signed-off-by: Rui Feng <rui_feng@...lsil.com.cn>
> > > > ---
> > > >   drivers/mfd/Makefile              |   4 +
> > > >   drivers/mfd/rtsx_pcr.c            | 127 ++++++-
> > > >   drivers/mfd/rtsx_pcr.h            |  12 +
> > > >   drivers/staging/Kconfig           |   2 +
> > > >   drivers/staging/rts5260/Kconfig   |   6 +
> > > >   drivers/staging/rts5260/rts5260.c | 748
> > ++++++++++++++++++++++++++++++++++++++
> > > >   drivers/staging/rts5260/rts5260.h |  45 +++
> > > >   include/linux/mfd/rtsx_pci.h      | 234 +++++++++++-
> > > >   8 files changed, 1173 insertions(+), 5 deletions(-)
> > > >   create mode 100644 drivers/staging/rts5260/Kconfig
> > > >   create mode 100644 drivers/staging/rts5260/rts5260.c
> > > >   create mode 100644 drivers/staging/rts5260/rts5260.h
> > 
> > [nearly 1000 lines snipped]
> > 
> > Wow, that's a lot of scrolling to get to your reply!
> > 
> > It would be helpful if you'd please snip your replies in the future.
> 
> Yes, sorry about that.
> 
> > 
> > > > There are a number of drivers in this family which currently reside in
> > > > MFD.  These were accepted before my time.  After a recent review I've
> > > > made the decision that these aren't MFD drivers at all.
> > >
> > > If all these other similar drivers don't belong in MFD, why are they
> > > still there?  Have they been moved in -next?
> > 
> > No effort has been made to move them yet.  We still don't have a
> > suitable home for them.  That's what we're discussing.
> 
> I see that on a part of this thread I wasn't on CC that drivers/misc
> was decided.
> 
> > 
> > > As recently as last month I still see patches being accepted into
> > > MFD regarding Realtek card readers.
> > >
> > > https://patchwork.kernel.org/patch/9941753/
> > 
> > Changes to existing drivers, yes.
> > 
> > I only noticed what was going on when this new driver was submitted.
> > 
> > I'm now not accepting new ones.
> > 
> > > I miss how it's reasonable to expect the submitter who is adding support
> > > for new HW that is similar to other hardware in the past to be able to
> > > know where to put it if this change hasn't already happened.
> > 
> > It's perfectly reasonable to reject a new driver which doesn't
> > belong.
> > 
> > > What's actually wrong with accepting it in MFD like the other drivers
> > > similar to it and then moving them all at once when the right place
> > > is figured out?
> > 
> > Because it removes the incentive for someone to take the initiative and
> > move them.  I can't work with promises.  Too many times I've heard "if
> > you just accept this patch, I'll do XYZ as a subsequent piece of
> > work", then move on.  They are either never seen again, or we have the
> > same conversation when they attempt to submit something else.  Things
> > don't get done that way.
> > 
> > > Then its much clearer for future drivers similar to this one that they
> > > live in the new place (misc, or wherever makes sense).
> > 
> > I've I would have accepted the original patch into MFD, we would not
> > be having this conversation, people like yourself and Greg would not
> > be aware and (the chances are - just going by previous experience
> > here) nothing would have changed/improved.
> > 
> > > It seems like it would be the same result but with less pain.
> > 
> > That cannot be guaranteed.
> > 
> > If people's words would have been their bond in the past, I would have
> > more trust and the world would be a nicer place. :)
> > 
> 
> Thanks for answering each of my points and explaining.  With strangers
> that you don't work with regularly, trust absolutely needs to built.
> 
> I noticed that Realtek submitted the driver to misc/ but didn't move
> the rest of the stuff in the series over the weekend.
> 
> I understand then that the ask would be to instead do 2 patch series:
> 1) Move the Realtek card reader drivers that are currently in 
> drivers/mfd/ over to drivers/misc.
> Move all the stuff in include/mfd headers related to these realtek
> devices over to a Realtek specific include/misc header
> 2) Submit this driver as into the new location.
> 
> I believe it should also be done relative to your -next tree since
> as I mentioned before there was some things already targeted at
> 4.15.
> 
> So if that aligns to your expectations I believe their submission into
> misc from this weekend should be discarded for now until they submit 
> the change to move the existing drivers as well.

Apologies for the delay, I was on vacation last week.

You're absolutely right.  Looks like the wheels are in motion:

  https://patchwork.kernel.org/patch/10015855/

Thanks for taking an interest.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Powered by blists - more mailing lists