Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC

[Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>]

On Wed, Feb 07, 2018 at 01:21:44PM +0000, A.s. Dong wrote:
> Hi Sascha,
> 
> > -----Original Message-----
> > From: Sascha Hauer [mailto:s.hauer@xxxxxxxxxxxxxx]
> > Sent: 2018年2月7日 19:12
> > To: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx>; A.s. Dong
> > <aisheng.dong@xxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>; Gary Bisson
> > <gary.bisson@xxxxxxxxxxxxxxxxxxx>; Vladimir Zapolskiy
> > <vladimir_zapolskiy@xxxxxxxxxx>; Sascha Hauer <kernel@xxxxxxxxxxxxxx>;
> > Mark Rutland <mark.rutland@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>;
> > open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> > <devicetree@xxxxxxxxxxxxxxx>; patchwork-lst@xxxxxxxxxxxxxx;
> > linux-gpio@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v2 1/3] dt-bindings: add binding for i.MX8MQ IOMUXC
> > 
> > On Wed, Feb 07, 2018 at 10:09:20AM +0100, Linus Walleij wrote:
> > > On Tue, Feb 6, 2018 at 4:47 PM, Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
> > >
> > > > To be fully honest I'm a bit annoyed with the pinctrl framework
> > > > making things (IMHO) unnecessarily complex, for what is basically a
> > > > pretty easy task.
> > >
> > > My ambition is to make things readable, understandable and
> > > maintainable. In generic terms, incorporating a bunch of knowledge of
> > > the electronics that really happen into the stuff we encode in the
> > > kernel.
> > >
> > > I guess it varies a bit on what goal one has.
> > >
> > > If the goal is "ship product with upstream kernel really fast now"
> > > then things like pinctrl-single.c where we just hammer magic values
> > > into registers, make sense. OMAP developers had no idea whatsoever
> > > what their ASIC people or cell library authors were doing so they just
> > > threw in the towel. HiSilicon also use this. Intels ambition was to
> > > use ACPI BIOS to handle all pin control and route around the kernel
> > > altogether, but that is not working out so well for them I think.
> > >
> > > All of them are approaches to avoid putting the hairy details into the
> > > kernel, just poke some magic values into some magic registers and be
> > > happy.
> > >
> > > So i.MX could have been like that, but then I guess you need to take
> > > legacy into account and discuss with the other i.MX driver authors
> > > about how they really wanted and want to do things.
> > >
> > > Their current silence wrt this mailchain is actually becoming a
> > > problem, and the problem is that discussing with you falls upwards to
> > > me as subsystem maintainer. Which sucks. I prefer that people who know
> > > this hardware discuss amongst themselves how they want things to work.
> > >
> > > Surely Sascha must have an opinion? It means much to me what he wants
> > > to do. I take it you guys are colleagues?
> > 
> > My opinion is that all that is generic about padctrl is a device driver saying "Put
> > my pins into a suitable mode". That is what padctrl is good for and we are there
> > for years now. I have always been happy with the plain register values in the
> > device tree. Before device tree we had exactly these values in the board files
> > and I never heard anyone complaining about it. There were defines for the bits
> > in the register which you could use when you were unhappy with plain register
> > values.
> > 
> > It's really trivial to look in the reference manual to make up the needed register
> > values. It's also trivial to take a register value and look into the reference
> > manual what this value does. Every translation layer, call it generic properties,
> > just makes things more complicated. Often enough our input is register value
> > tables from either our customers our from spreadsheets from FSL/NXP. Every
> > translation layer in the way just means we have to translate the already existing
> > register values into something hoping that this correctly translates back into the
> > register values.
> > 
> > It's not that some board designer comes up with "I need a drive strength of
> > 150mA" and wants to put that value into the device tree. Instead they start
> > with the reference manual, see which values they can (must) adjust and then
> > adjust the values until they are happy. No one wants to ask questions like "How
> > do I have to manipulate that device tree to change that particular bit?"
> > 
> > As said, I am happy with plain register values in the device tree and I consider
> > everything else overengineered.
> > FSL/NXP Reference Manuals are freely available and of high quality so everybody
> > can understand the register values. There's nothing magic to them. That might
> > change slightly when the Manuals are not available, but even then I think that
> > not the device tree ABI is the right place to add that missing documentation.
> > 
> 
> Probably as I first implemented generic pinconfig support for i.MX, personally
> I'm tend to it a bit, mainly because below reasons we find in real situations:
> 
> First and most important, plain register value is a bit error prone.
> (Actually we meet many times of issues during internal development).
> User has to compose it manually by refer to reference manual
> which might be more easy to mistake than using standard binding property.
> 
> And not everyone would like to compose the register value manually
> when writing their device tree, they may simply do copy & paste of an exist one
> and quickly test, if it works, everything is done. They don't know which setting is actually
> used as they can see nothing from the plain register value. Untill the device becomes
> unwork in some special circumstances, then they have to waste a lot time to find out
> the pad setting issue. This happens especially more often when switch to
> a new SoC while the pad config register layout changed a bit.
> We certainly would like to avoid it!
> 
> Secondly, plain register value is also unreadable and unmaintainable,
> also not quite friendly in reviewing other one's patches.
> 
> Last, it might be trivial, people don't want to refer to reference manual
> again and again once being asked which driver strength or something else
> Is used.

These issues could be solved by bitmask defines aswell, no need to bloat
the dtbs with strings.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html