RE: staging: rtsx: Add support for RTS5260

Mario.Limonciello
2017-10-16T20:38:32Z

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