Hi, > Am 20.09.2016 um 23:45 schrieb Tony Lindgren <[email protected]>: > > * H. Nikolaus Schaller <[email protected]> [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 <[email protected]>: >>> >>> * H. Nikolaus Schaller <[email protected]> [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
