From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753992AbcIUEYD (ORCPT ); Wed, 21 Sep 2016 00:24:03 -0400 Received: from mo4-p00-ob.smtp.rzone.de ([81.169.146.218]:21309 "EHLO mo4-p00-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750858AbcIUEYB (ORCPT ); Wed, 21 Sep 2016 00:24:01 -0400 X-RZG-AUTH: :JGIXVUS7cutRB/49FwqZ7WcecEarQROEYabkiUo6mSAGQ+qKID8oJ3kG X-RZG-CLASS-ID: mo00 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Subject: Re: problem with pinctrl-single,bits and control_devconf0 From: "H. Nikolaus Schaller" In-Reply-To: <20160920214510.vd25yapmhlgipfv4@atomide.com> Date: Wed, 21 Sep 2016 06:23:55 +0200 Cc: Keerthy , Discussions about the Letux Kernel , LKML Message-Id: References: <20160919180709.qbcnc6hnojjxgmxr@atomide.com> <7F3DB90C-5001-47D2-B38B-98A81506BD46@goldelico.com> <20160920214510.vd25yapmhlgipfv4@atomide.com> To: Tony Lindgren X-Mailer: Apple Mail (2.3124) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u8L4O7vC004654 Hi, > Am 20.09.2016 um 23:45 schrieb Tony Lindgren : > > * H. Nikolaus Schaller [160920 01:44]: >> Hi Tony, >> (I had forgotten to cc LKML so I have added it). >> >>> Am 19.09.2016 um 20:07 schrieb Tony Lindgren : >>> >>> * H. Nikolaus Schaller [160918 06:53]: >>>> Hi, >>>> I am trying to set up a special McBSP1 CLKR on OMAP3, but I don't understand >>>> the logic of the offsets and masks behind pinctrl-single,bits. >>>> >>>> I have modified the example given in the bindings document >>>> >>>> http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt?v=3.8#L110 >>>> >>>> to: >>>> >>>> / { >>>> /* pinmux for devconf0 */ >>>> control_devconf0: pinmux@48002274 { >>>> compatible = "pinctrl-single"; >>>> reg = <0x48002274 4>; /* Single register */ >>>> #address-cells = <1>; >>>> #size-cells = <0>; >>>> pinctrl-single,bit-per-mux; >>>> pinctrl-single,register-width = <32>; >>>> pinctrl-single,function-mask = <0x5F>; >>>> }; >>>> }; >>>> >>>> &control_devconf0 { >>>> pinctrl-names = "default"; >>>> pinctrl-0 = <&mcbsp1_defconf0>; >>>> >>>> mcbsp1_defconf0: pinmux_mcbsp1_defconf0 { >>>> /* offset bits mask */ >>>> pinctrl-single,bits = <0x00 0x08 0x08>; /* set MCBSP1_CLKR_MASK */ >>>> }; >>>> }; >>>> >>>> This looks reasonable to me to set bit 0x08 of the DEVCONF0 register which is within the set of the bits "enabled" for modification by the mask 0x5F. >>> >>> Yes as long as devconf0 is purely a mux register with no extra features >>> like regulators or clocks. >>> >>>> All I get from this is a reject: >>>> >>>> [ 1.808258] pinctrl-single 48002274.pinmux: Invalid submask 0x8 for pinmux_mcbsp1_defconf0 at 0x0 >>>> >>>> So what is wrong? >>>> >>>> I have added some printk to drivers/pinctrl/pinctrl-single.c to understand what >>>> the bits, offsets and masks are thought to do. >>>> >>>> [ 1.807220] pinctrl-single: fmask=0000005f fshift=0 fmax=0000005f >>>> [ 1.807464] pinctrl-single: pcs->fmask = 0000005f >>>> [ 1.807495] pinctrl-single: mask = 00000008 bit_pos = 3 pin_num_from_lsb = 0 mask_pos = 000002f8 val_pos = 00000008 submask = 00000008 >>>> [ 1.807495] pinctrl-single: -> mask = 00000000 >>>> [ 1.807525] pinctrl-single 48002274.pinmux: Invalid submask 0x8 for pinmux_mcbsp1_defconf0 at 0x0 >>>> >>>> What I don't understand at all is why the mask 0x5f gives a mask_pos = 000002f8. >>>> Of course then the mask can never be the same as the submask. >>>> >>>> Or is the example in the bindings document wrong? >>> >>> Hmm can't say what might be wrong, it is pretty widely used for da850 and >>> keystone. Maybe check those dtsi files and see if there's some difference >>> compared to the docs? >> >> Yes, that was a good hint. >> >> I found how it is used in da850.dtsi and it looks as if the documentation is >> too sparse and the example is wrong. >> >> The reason seems to be that pinctrl-single,function-mask has changed its >> role some time ago. At least >> >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=055cb2a9e065c3a606f57fbcf8de1689f7c1fedf >> >> seems to indicate that. >> >> My current working hypothesis is that function-mask no longer specifies >> which bits *can* be changed in total (depending on the individual mask) >> but which bits *must* be changed by a single mask. >> >> So the old one was to reject setting bits that are not to be changed while >> the new one enforces several bits to be changed as a group. >> >> 0x5f did mean: you can change any of these 5 bits but with the modified >> scheme it seems to mean: you must set/reset all these bits in parallel. >> >> Hence setting just one bit (my example) or two bits (bindings document >> example) is rejected. >> >> This did not at all become clear to me from the bindings description... >> >> I have tried both: >> >>>> pinctrl-single,function-mask = <0x01>; >>>> pinctrl-single,bits = <0x00 0x08 0x08>; >> >>>> pinctrl-single,function-mask = <0x5F>; >>>> pinctrl-single,bits = <0x00 0x08 0x5f>; >> >> and both are not rejected and set the expected value. > > OK, care to update the doc while at it so we can avoid running > into this again? Yes, I'd like to. The problem is that I need some confirmation that my understanding is really correct... I just did guess and succeed. So someone who understands the driver code should comment first. Then I can prepare a patch for the documentation. BR, Nikolaus