* Re: problem with pinctrl-single,bits and control_devconf0 [not found] ` <20160919180709.qbcnc6hnojjxgmxr@atomide.com> @ 2016-09-20 8:43 ` H. Nikolaus Schaller 2016-09-20 21:45 ` Tony Lindgren 0 siblings, 1 reply; 3+ messages in thread From: H. Nikolaus Schaller @ 2016-09-20 8:43 UTC (permalink / raw) To: Tony Lindgren; +Cc: Keerthy, Discussions about the Letux Kernel, LKML, Prakash Hi Tony, (I had forgotten to cc LKML so I have added it). > Am 19.09.2016 um 20:07 schrieb Tony Lindgren <tony@atomide.com>: > > * H. Nikolaus Schaller <hns@goldelico.com> [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. BR and thanks, Nikolaus ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: problem with pinctrl-single,bits and control_devconf0 2016-09-20 8:43 ` problem with pinctrl-single,bits and control_devconf0 H. Nikolaus Schaller @ 2016-09-20 21:45 ` Tony Lindgren 2016-09-21 4:23 ` H. Nikolaus Schaller 0 siblings, 1 reply; 3+ messages in thread From: Tony Lindgren @ 2016-09-20 21:45 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Keerthy, Discussions about the Letux Kernel, LKML, Prakash * H. Nikolaus Schaller <hns@goldelico.com> [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 <tony@atomide.com>: > > > > * H. Nikolaus Schaller <hns@goldelico.com> [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? Regards, Tony ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: problem with pinctrl-single,bits and control_devconf0 2016-09-20 21:45 ` Tony Lindgren @ 2016-09-21 4:23 ` H. Nikolaus Schaller 0 siblings, 0 replies; 3+ messages in thread From: H. Nikolaus Schaller @ 2016-09-21 4:23 UTC (permalink / raw) To: Tony Lindgren; +Cc: Keerthy, Discussions about the Letux Kernel, LKML Hi, > Am 20.09.2016 um 23:45 schrieb Tony Lindgren <tony@atomide.com>: > > * H. Nikolaus Schaller <hns@goldelico.com> [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 <tony@atomide.com>: >>> >>> * H. Nikolaus Schaller <hns@goldelico.com> [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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-09-21 4:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <EEC747D6-D13B-49E3-80D0-58E5A5C96C61@goldelico.com>
[not found] ` <20160919180709.qbcnc6hnojjxgmxr@atomide.com>
2016-09-20 8:43 ` problem with pinctrl-single,bits and control_devconf0 H. Nikolaus Schaller
2016-09-20 21:45 ` Tony Lindgren
2016-09-21 4:23 ` H. Nikolaus Schaller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®