LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Amelie DELAUNAY <amelie.delaunay@st.com>
Cc: Lee Jones <lee.jones@linaro.org>, Rob Herring <robh@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Russell King <linux@armlinux.org.uk>,
	Alexandre TORGUE <alexandre.torgue@st.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/5] dt-bindings: pinctrl: document the STMFX pinctrl bindings
Date: Thu, 24 May 2018 09:13:32 +0200	[thread overview]
Message-ID: <CACRpkdbZ2Hr7cc5+dyPaL0CKYuy2-oosEr=UPo60+3beDCVi9A@mail.gmail.com> (raw)
In-Reply-To: <28c374e6-b440-f785-b371-03fe15c8bc4b@st.com>

On Fri, May 18, 2018 at 9:29 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
> On 05/17/2018 08:36 AM, Lee Jones wrote:
>> On Wed, 16 May 2018, Amelie DELAUNAY wrote:
>>> On 05/16/2018 04:20 PM, Linus Walleij wrote:
>>>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
>>>>
>>>>> Indeed, stmfx has other functions than GPIO. But, after comments done
>>>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO
>>>>> child drivers into a single PINCTRL/GPIO driver because of the following
>>>>> reasons:
>>>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are
>>>>> not used on any of the boards using an stmfx and supported by Linux, so
>>>>> no way to test these functions, and no need to maintain them while they
>>>>> are not being used.
>>>>> - But, in the case a new board will use more than GPIO function on
>>>>> stmfx, the actual implementation allow to easily extract common init
>>>>> part of stmfx and put it in an MFD driver.
>>>>>
>>>>> So I could remove gpio sub-node and put its contents in stmfx node and
>>>>> keep single PINCTRL/GPIO driver for the time being.
>>>>> Please advise,
>>>>
>>>> I would normally advice to use the right modeling from the start, create
>>>> the MFD driver and spawn the devices from there. It is confusing
>>>> if the layout of the driver(s) doesn't really match the layout of the
>>>> hardware.
>>>>
>>>> I understand that it is a pain to write new MFD drivers to get your
>>>> things going and it would be "nice to get this working really quick
>>>> now" but in my experience it is better to do it right from the start.
>>>>
>>>
>>> Hi Linus,
>>>
>>> Thanks for your advice. I understand the point.
>>> So, the right modeling would be to:
>>> - create an MFD driver with the common init part of stmfx
>>> - remove all common init part of stmfx-pinctrl driver and keep only all
>>> gpio/pinctrl functions.
>>>
>>> I will not develop the other stmfx functions (IDD measurement driver and
>>> TouchScreen controller driver) because, as explained ealier, they are
>>> not used on any of the boards using an stmfx and supported by Linux, so
>>> no way to test these functions, and no need to maintain them while they
>>> are not being used.
>>>
>>> Lee, are you OK with that ?
>>
>> I missed a lot of this conversation I think, but from what I've read,
>> it sounds fine.
>>
>
> I summarize the situation:
> - I still don't have an official datasheet for STMFX device which could
> justify the use of an MFD driver;
> - the MFD driver will contain the STMFX chip initialization stuff such
> as regmap initialization (regmap structure will be shared with the
> child), chip initialization, global interrupt management;
> - there will be only one child (GPIO/PINCTRL node) for the time being.

But there will be more devices in it. And they will invariably be put
to use later, and there will be new versions of the chip as well, and
then you will be happy about doing the MFD core, which makes it
easy to add new variants with different subdevices.

> So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does
> it still sound fine after this summary ? :)

No I think it should use an MFD core.

Mainly because of device tree concerns.

The main reason is that the device tree bindings will be different if
you add an MFD core later, the GPIO and pinctrl driver will
move to a child node, making old device trees incompatible.

We could have a single driver in GPIO+pin control if it is a child
of an MFD node in the device tree, but it doesn't make much
sense as the I2C device need to be probing to the MFD core.

Yours,
Linus Walleij

  parent reply	other threads:[~2018-05-24  7:13 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-11  9:47 [PATCH 0/5] Introduce STMFX I2C GPIO expander Amelie Delaunay
2018-04-11  9:47 ` [PATCH 1/5] dt-bindings: pinctrl: document the STMFX pinctrl bindings Amelie Delaunay
2018-04-16 18:19   ` Rob Herring
2018-04-26 12:49     ` Linus Walleij
2018-05-09  7:56       ` Amelie DELAUNAY
2018-05-16 14:20         ` Linus Walleij
2018-05-16 15:01           ` Amelie DELAUNAY
2018-05-17  6:36             ` Lee Jones
2018-05-18  7:29               ` Amelie DELAUNAY
2018-05-18 13:52                 ` Lee Jones
2018-05-18 15:13                   ` Amelie DELAUNAY
2018-05-24  7:13                 ` Linus Walleij [this message]
2018-05-28 11:39                   ` Amelie DELAUNAY
2018-05-29  7:36                     ` Lee Jones
2018-05-09  7:31     ` Amelie DELAUNAY
2018-04-11  9:47 ` [PATCH 2/5] pinctrl: Add STMFX GPIO expander Pinctrl/GPIO driver Amelie Delaunay
2018-04-12  3:11   ` kbuild test robot
2018-04-26 12:48   ` Linus Walleij
2018-05-09  9:13     ` Amelie DELAUNAY
2018-04-11  9:47 ` [PATCH 3/5] ARM: dts: stm32: add STMFX pinctrl/gpio expander support on stm32746g-eval Amelie Delaunay
2018-04-11  9:47 ` [PATCH 4/5] ARM: dts: stm32: add orange and blue leds " Amelie Delaunay
2018-04-11  9:47 ` [PATCH 5/5] ARM: dts: stm32: add joystick support " Amelie Delaunay

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='CACRpkdbZ2Hr7cc5+dyPaL0CKYuy2-oosEr=UPo60+3beDCVi9A@mail.gmail.com' \
    --to=linus.walleij@linaro.org \
    --cc=alexandre.torgue@st.com \
    --cc=amelie.delaunay@st.com \
    --cc=devicetree@vger.kernel.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=robh@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).