LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Arnd Bergmann <arnd@arndb.de>, Rob Herring <robh+dt@kernel.org>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Grant Likely <grant.likely@linaro.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board
Date: Tue, 17 Feb 2015 07:43:10 +0000	[thread overview]
Message-ID: <20150217074310.GT14545@x1> (raw)
In-Reply-To: <87fva5a4i2.fsf@free.fr>

Arnd, Greg,

  Perhaps you have some ideas WRT programmables (PLDs/CPLDs/FPGAs)?

On Mon, 16 Feb 2015, Robert Jarzmik wrote:
> Lee Jones wrote:
> > What's all this?  Please configure your mail client correctly.
> >
> > For advice, see:
> >
> >   Documentation/email-clients.txt
> While at day work, I have only access to web mail ...

Probably best to hold off your reply until you get home then.  It's
not just a formatting issue, you also exposed many raw email addresses
to the internet, which are easily harvested up by web crawlers.

See this:
  https://lkml.org/lkml/2015/2/16/247

> >>  2) after v2, we _both_ agreed that the accurate name is "cplds"
> >>     which exactly what is in this patch
> >>     (see device registering with lubbock_cplds).
> >
> > There is no mention of this decision in the changelogs.  If it's not
> > in the change logs, it didn't happen. ;)
> Ah right.
> 
> > I'm still concerned with the fact that the driver file is named using
> > and is populated by lots of instances of a "board" name.  I'm sure you
> > would share my thoughts is someone submitted a driver called
> > beaglebone.c or raspberrypi.c to MFD.
> I understand your concern. Arnd, a thought about it ? The only viable
> alternative would be to move it to arch/arm/plat-pxa AFAIS.

I don't think this is correct either.  CPLD handling would probably be
slightly less out of place in drivers/misc, but perhaps a new
subsystem for PLDs/CPLDs/FPGAs would be more appropriate
drivers/programmables or similar maybe.

> >> > Besides, this is MFD, where we support single pieces of silicon which
> >> > happen to support multiple devices.  I definitely don't want to support
> >> > boards here.
> >> > You might want to re-think the naming and your (sales) pitch.
> >> I might need help. As for the (sales), no comment.
> >
> > By pitch, I mean to convince me that this belongs in MFD.
> I've been trying.

I'm pretty convinced that it doesn't belong in MFD now, but it doesn't
mean I'm going to leave you on the curb.  I'd like to help you get it
into a better home.

[...]

> > Understanding was lost when I learned that the whole file was centred
> > around the premise of board support.  I understand now that this is a
> > driver for a CPLD, which I'm not sure is even MFD.  Also, if it is
> > really a CPLD driver, shouldn't be named after the manufacturer/model
> > number of the chip, rather than the PCB which it's connected to?
> >
> > I'm also concerned that this driver has no functional CPLD code
> > contained.  All you're doing is defining an IRQ domain.  Why then
> > isn't this really an drivers/irqchip (Suggested-by: Josh Cartwright)
> > driver?
> Is not only a irqchip because, as explained at the bottom of the commit message,
> quoting myself :
>   This patch moves all that handling to a mfd driver. It's only purpose
>   for the time being is the interrupt handling, but in the future it
>   should encompass all the motherboard CPLDs handling :
>    - leds
>    - switches
>    - hexleds

I had a conversation about this on IRC yesterday and some good
points/questions were posed.  This is a difficult area, because you
can program these things to do whatever you like.  Depending on the
'intention' (and it is only an intention -- someone else can come
along and reprogram these devices on a whim), the CPLD code could live
anywhere.  If you wanted to put watchdog functionality in there, then
there is an argument for it to live in drivers/watchdog, etc etc.  So
just because the plan is to support a few (i.e. more than one) simple
devices, it doesn't necessarily mean that the handling should be done
in MFD.

Yesterday I was asked "Are you wanting to restrict drivers in
drivers/mfd to those that make use of MFD_CORE functionality?".  My
answer to that was "No, however; I only want devices which
_intrinsically_ operate in multiple subsystems", which these
programmables no not do.

FYI, you're not on your own here.  There is at least one of these
devices in the kernel already and upon a short inspection there
appears to be a number of Out-of-Tree (OoT) drivers out there which
will require a home in Mainline sooner or later.

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

  reply	other threads:[~2015-02-17  7:43 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-24 15:05 [PATCH v4 1/4] dt-bindings: mfd: add lubbock-cplds binding Robert Jarzmik
2015-01-24 15:05 ` [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board Robert Jarzmik
2015-02-16 13:05   ` Lee Jones
2015-02-16 13:27     ` robert.jarzmik
2015-02-16 16:27       ` Lee Jones
2015-02-16 22:14         ` Robert Jarzmik
2015-02-17  7:43           ` Lee Jones [this message]
2015-02-17 17:38             ` Nicolas Pitre
2015-02-18  8:07               ` Lee Jones
     [not found]                 ` <87a9088tam.fsf@free.fr>
2015-02-28  9:57                   ` Robert Jarzmik
2015-02-28 15:11                     ` Greg Kroah-Hartman
2015-02-28 15:29                       ` Robert Jarzmik
2015-03-25 14:07                   ` Greg Kroah-Hartman
2015-03-26 21:38                     ` Robert Jarzmik
2015-03-26 23:47                       ` Greg Kroah-Hartman
2015-03-28  2:35                       ` Arnd Bergmann
2015-03-28  8:29                         ` Robert Jarzmik
2015-03-28 13:24                           ` Arnd Bergmann
2015-01-24 15:05 ` [PATCH v4 3/4] ARM: pxa: lubbock: use new lubbock_cplds driver Robert Jarzmik
2015-01-24 15:05 ` [PATCH v4 4/4] MAINTAINERS: add entry for lubbock-cplds Robert Jarzmik
2015-02-10 18:41 ` [PATCH v4 1/4] dt-bindings: mfd: add lubbock-cplds binding Robert Jarzmik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150217074310.GT14545@x1 \
    --to=lee.jones@linaro.org \
    --cc=arnd@arndb.de \
    --cc=daniel@zonque.org \
    --cc=dbaryshkov@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=grant.likely@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robert.jarzmik@free.fr \
    --cc=robh+dt@kernel.org \
    --cc=sameo@linux.intel.com \
    --subject='Re: [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).