Re: staging: rtsx: Add support for RTS5260

[Lee Jones]

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.

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

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

> > Ok, how does it hook up to the hardware to talk to the reader?
> 
> This particular case its PCIe.  I don't know if their chip is
> used in any other packages, but I wouldn't be surprised if so.

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