LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: robert.jarzmik@free.fr
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,
	Arnd Bergmann <arnd@arndb.de>,
	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 16:27:12 +0000	[thread overview]
Message-ID: <20150216162712.GR14545@x1> (raw)
In-Reply-To: <1170550770.470129293.1424093248596.JavaMail.root@zimbra1-e1.priv.proxad.net>

On Mon, 16 Feb 2015, robert.jarzmik@free.fr wrote:

> ----- Mail original -----
> De: "Lee Jones" <lee.jones@linaro.org>
> À: "Robert Jarzmik" <robert.jarzmik@free.fr>
> 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, "Arnd Bergmann" <arnd@arndb.de>, "Russell King - ARM Linux" <linux@arm.linux.org.uk>, "Dmitry Eremin-Solenikov" <dbaryshkov@gmail.com>
> Envoyé: Lundi 16 Février 2015 14:05:49
> Objet: Re: [PATCH v4 2/4] mfd: lubbock_cplds: add lubbock IO board

What's all this?  Please configure your mail client correctly.

For advice, see:

  Documentation/email-clients.txt

> On Sat, 24 Jan 2015, Robert Jarzmik wrote:
> 
> > ---
> > Since v1: change the name from cottula to lubbock_io
> >             Dmitry pointed out the Cottula was the pxa25x family name,
> > 	    lubbock was the pxa25x development board name. Therefore the
> > 	    name was changed to lubbock_io (lubbock IO board)
> 
> > Are you sure this is what you want to do?  We don't usually support
> > 'boards' per say.  Instead we support 'devices', then pull each of
> > those devices together using some h/w description mechanism.
> 
> Do you know that :
>  1) anything under "---" in a commit message is thrown away

Yes.  But I don't remember reading this passage before.  Just becuase
this is no longer v2, it doesn't void any comments regarding v1
changelogs.

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

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.

>  3) there is no more mention of "board" anywhere in the patch core

No, just the name of one.

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

> >> +#include <linux/mfd/core.h>
> > Why have you included this?  I don't see the use of the MFD framework
> > anywhere.  So what makes this an MFD?
> 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.

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

-- 
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-16 16:27 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 [this message]
2015-02-16 22:14         ` Robert Jarzmik
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=20150216162712.GR14545@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=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).