RE: staging: rtsx: Add support for RTS5260

[Mario.Limonciello]

> -----Original Message-----
> From: Lee Jones [mailto:lee.jones@xxxxxxxxxx]
> Sent: Friday, October 13, 2017 4:15 PM
> To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx>
> Cc: rui_feng@xxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devel@xxxxxxxxxxxxxxxxxxxx; ricky_wu@xxxxxxxxxxx;
> wei_wang@xxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxx wrote:
> > > From: rui_feng <rui_feng@xxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxx>
> > > ---
> > >   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.