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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Regards
Dong Aisheng

> Sascha
> 
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
> engutronix.de%2F&data=02%7C01%7Caisheng.dong%40nxp.com%7C2644d871
> 2e234a6c2e7a08d56e1b9b29%7Cbd8a2a2207224ec7b35f1c4f0497e341%7C0%
> 7C0%7C636535987211269744&sdata=vImuWxtT7Ya0uxEHrnROUYZ%2BiCWLs8
> LmxWB5oNPcgjk%3D&reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |
?韬{.n?????%??檩??w?{.n????z谵{???塄}?财??j:+v??????2??璀??摺?囤??z夸z罐?+?????w棹f




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux