staging: rtsx: Add support for RTS5260

Mario.Limonciello at dell.com Mario.Limonciello at dell.com
Mon Oct 16 20:38:27 UTC 2017


> -----Original Message-----
> From: Lee Jones [mailto:lee.jones at linaro.org]
> Sent: Friday, October 13, 2017 4:15 PM
> To: Limonciello, Mario <Mario_Limonciello at Dell.com>
> Cc: rui_feng at realsil.com.cn; gregkh at linuxfoundation.org; linux-
> kernel at vger.kernel.org; devel at driverdev.osuosl.org; ricky_wu at realtek.com;
> wei_wang at realsil.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 at realsil.com.cn wrote:
> > > From: rui_feng <rui_feng at realsil.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 at realsil.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.



More information about the devel mailing list