LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Robert Jarzmik <robert.jarzmik@free.fr>
To: Lee Jones <lee.jones@linaro.org>, Arnd Bergmann <arnd@arndb.de>
Cc: 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>
Subject: Re: [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board
Date: Mon, 16 Feb 2015 23:14:13 +0100	[thread overview]
Message-ID: <87fva5a4i2.fsf@free.fr> (raw)
In-Reply-To: <20150216162712.GR14545@x1> (Lee Jones's message of "Mon, 16 Feb 2015 16:27:12 +0000")

Lee Jones <lee.jones@linaro.org> writes:

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

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

>> > 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 thought cplds were to be handled by an MFD driver.
>
> Not to my knowledge, although we do appear to have one.  That doesn't
> necessarily mean that it's the right place for it though.  I'm not
> entirely sure how CPLDs even work on a functional level.
As I said, I understand your concern.

>> > I'm going to stop here, as I think I need more of an explanation so
>> > what you're trying to achieve with this driver.
>> Why ? I think things were clear that this driver handles the CPLDs on
>> lubbock board, namely u46 and u52. I don't understand what is wrong
>> with this patch so that you don't want to go forward.
>
> 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

When these parts will be added, and they'll have to be handled by this driver
because of iospace sharing, and same IP (cplds) providing the hardware support,
the irqchip way will be just impossible to follow.

As I said above, I understand your concern. If we all reach a concensus this
should be moved to the pxa tree, so be it. But I'd like other opinion here.

Cheers.

-- 
Robert

  reply	other threads:[~2015-02-16 22:14 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 [this message]
2015-02-17  7:43           ` Lee Jones
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=87fva5a4i2.fsf@free.fr \
    --to=robert.jarzmik@free.fr \
    --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=haojian.zhuang@gmail.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=lee.jones@linaro.org \
    --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=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).