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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
