Re: staging: rtsx: Add support for RTS5260

Lee Jones
2017-10-23T09:18:40Z

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

> > -----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.

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