Query on pinctrl usage for DT nodes

Query on pinctrl usage for DT nodes

[Prabhakar Lad]

Hi Linus/Stephen,

I am working adding  DT nodes for DA850.

Following is the pin control snippet of da850.dtsi:-

		pmx_core: pinmux@1c14120 {
			compatible = "pinctrl-single";
			reg = <0x14120 0x50>;
			#address-cells = <1>;
			#size-cells = <0>;
			pinctrl-single,bit-per-mux;
			pinctrl-single,register-width = <32>;
			pinctrl-single,function-mask = <0xffffffff>;
			status = "disabled"

                        i2c0_pins: pinmux_i2c0_pins {
                                pinctrl-single,bits = <
                                        /* I2C0_SDA,I2C0_SCL */
                                        0x10 0x00002200 0x0000ff00
                                >;
                        };
                        mdio_pins: pinmux_mdio_pins {
                                pinctrl-single,bits = <
                                        /* MDIO_CLK, MDIO_D */
                                        0x10 0x00000088 0x000000ff
                                >;
                        };
                }

And following is the code snippet for dts:-

		i2c0: i2c@1c22000 {
			status = "okay";
			clock-frequency = <100000>;
			pinctrl-names = "default";
			pinctrl-0 = <&i2c0_pins>;
		};
		mdio: mdio@1e24000 {
			status = "okay";
			bus_freq = <2200000>;
			pinctrl-names = "default";
              };
		
But while booting I see the following boot log:-
...
cpuidle: using governor menu
TCP: cubic registered
NET: Registered protocol family 17
pinctrl-single 1c14120.pinmux: pin 1c14130 already requested by
davinci_mdio.0; cannot claim for i2c_davinci.1
pinctrl-single 1c14120.pinmux: pin-4 (i2c_davinci.1) status -22
pinctrl-single 1c14120.pinmux: could not request pin 4 on device pinctrl-single
console [netcon0] enabled
....

This is because the mdio and i2c are using same pin 0x10, Is there any
alternative way to handle if the two node's are using same pins any
pointers could be very much helpful ?

Regards,
--Prabhakar

Re: Query on pinctrl usage for DT nodes

[Stephen Warren]

On 04/03/2013 03:16 AM, Prabhakar Lad wrote:
> Hi Linus/Stephen,
> 
> I am working adding  DT nodes for DA850.
> 
> Following is the pin control snippet of da850.dtsi:-
> 
> 		pmx_core: pinmux@1c14120 {
...
>                         i2c0_pins: pinmux_i2c0_pins {
>                                 pinctrl-single,bits = <
>                                         /* I2C0_SDA,I2C0_SCL */
>                                         0x10 0x00002200 0x0000ff00
>                                 >;
>                         };
>                         mdio_pins: pinmux_mdio_pins {
>                                 pinctrl-single,bits = <
>                                         /* MDIO_CLK, MDIO_D */
>                                         0x10 0x00000088 0x000000ff
>                                 >;
>                         };
>                 }
> 
> And following is the code snippet for dts:-
> 
> 		i2c0: i2c@1c22000 {
> 			status = "okay";
> 			clock-frequency = <100000>;
> 			pinctrl-names = "default";
> 			pinctrl-0 = <&i2c0_pins>;
> 		};
> 		mdio: mdio@1e24000 {
> 			status = "okay";
> 			bus_freq = <2200000>;
> 			pinctrl-names = "default";

I assume there's also the following there:

			pinctrl-0 = <&mdio_pins>;

>               };
> 		
> But while booting I see the following boot log:-
> ...
> cpuidle: using governor menu
> TCP: cubic registered
> NET: Registered protocol family 17
> pinctrl-single 1c14120.pinmux: pin 1c14130 already requested by
> davinci_mdio.0; cannot claim for i2c_davinci.1
> pinctrl-single 1c14120.pinmux: pin-4 (i2c_davinci.1) status -22
> pinctrl-single 1c14120.pinmux: could not request pin 4 on device pinctrl-single
> console [netcon0] enabled
> ....
> 
> This is because the mdio and i2c are using same pin 0x10,

How can two devices use the same pin? I mean physically, in hardware?

Is this because pinctrl-single uses the register address as the pin
number, whereas you have registers which configure multiple pins at
once? If so, your hardware isn't something that can be represented by
pinctrl-single.

> Is there any
> alternative way to handle if the two node's are using same pins any
> pointers could be very much helpful ?

Re: Query on pinctrl usage for DT nodes

[Tony Lindgren]

* Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> [130403 09:49]:
> On 04/03/2013 03:16 AM, Prabhakar Lad wrote:
> > Hi Linus/Stephen,
> > 
> > I am working adding  DT nodes for DA850.
> > 
> > Following is the pin control snippet of da850.dtsi:-
> > 
> > 		pmx_core: pinmux@1c14120 {
> ...
> >                         i2c0_pins: pinmux_i2c0_pins {
> >                                 pinctrl-single,bits = <
> >                                         /* I2C0_SDA,I2C0_SCL */
> >                                         0x10 0x00002200 0x0000ff00
> >                                 >;
> >                         };
> >                         mdio_pins: pinmux_mdio_pins {
> >                                 pinctrl-single,bits = <
> >                                         /* MDIO_CLK, MDIO_D */
> >                                         0x10 0x00000088 0x000000ff
> >                                 >;
> >                         };
> >                 }
> > 
> > And following is the code snippet for dts:-
> > 
> > 		i2c0: i2c@1c22000 {
> > 			status = "okay";
> > 			clock-frequency = <100000>;
> > 			pinctrl-names = "default";
> > 			pinctrl-0 = <&i2c0_pins>;
> > 		};
> > 		mdio: mdio@1e24000 {
> > 			status = "okay";
> > 			bus_freq = <2200000>;
> > 			pinctrl-names = "default";
> 
> I assume there's also the following there:
> 
> 			pinctrl-0 = <&mdio_pins>;
> 
> >               };
> > 		
> > But while booting I see the following boot log:-
> > ...
> > cpuidle: using governor menu
> > TCP: cubic registered
> > NET: Registered protocol family 17
> > pinctrl-single 1c14120.pinmux: pin 1c14130 already requested by
> > davinci_mdio.0; cannot claim for i2c_davinci.1
> > pinctrl-single 1c14120.pinmux: pin-4 (i2c_davinci.1) status -22
> > pinctrl-single 1c14120.pinmux: could not request pin 4 on device pinctrl-single
> > console [netcon0] enabled
> > ....
> > 
> > This is because the mdio and i2c are using same pin 0x10,
> 
> How can two devices use the same pin? I mean physically, in hardware?
> 
> Is this because pinctrl-single uses the register address as the pin
> number, whereas you have registers which configure multiple pins at
> once? If so, your hardware isn't something that can be represented by
> pinctrl-single.
> 
> > Is there any
> > alternative way to handle if the two node's are using same pins any
> > pointers could be very much helpful ?

It could also that the mux register(s) follow the one-mux-per-bit
mapping.

In that case pinctrl-single,bits option as documented in the
Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt.

Regards,

Tony

Re: Query on pinctrl usage for DT nodes

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [130403 11:42]:
> * Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> [130403 09:49]:
> > On 04/03/2013 03:16 AM, Prabhakar Lad wrote:
> > > Hi Linus/Stephen,
> > > 
> > > I am working adding  DT nodes for DA850.
> > > 
> > > Following is the pin control snippet of da850.dtsi:-
> > > 
> > > 		pmx_core: pinmux@1c14120 {
> > ...
> > >                         i2c0_pins: pinmux_i2c0_pins {
> > >                                 pinctrl-single,bits = <
> > >                                         /* I2C0_SDA,I2C0_SCL */
> > >                                         0x10 0x00002200 0x0000ff00
> > >                                 >;
> > >                         };
> > >                         mdio_pins: pinmux_mdio_pins {
> > >                                 pinctrl-single,bits = <
> > >                                         /* MDIO_CLK, MDIO_D */
> > >                                         0x10 0x00000088 0x000000ff
> > >                                 >;
> > >                         };
> > >                 }
> > > 
> > > And following is the code snippet for dts:-
> > > 
> > > 		i2c0: i2c@1c22000 {
> > > 			status = "okay";
> > > 			clock-frequency = <100000>;
> > > 			pinctrl-names = "default";
> > > 			pinctrl-0 = <&i2c0_pins>;
> > > 		};
> > > 		mdio: mdio@1e24000 {
> > > 			status = "okay";
> > > 			bus_freq = <2200000>;
> > > 			pinctrl-names = "default";
> > 
> > I assume there's also the following there:
> > 
> > 			pinctrl-0 = <&mdio_pins>;
> > 
> > >               };
> > > 		
> > > But while booting I see the following boot log:-
> > > ...
> > > cpuidle: using governor menu
> > > TCP: cubic registered
> > > NET: Registered protocol family 17
> > > pinctrl-single 1c14120.pinmux: pin 1c14130 already requested by
> > > davinci_mdio.0; cannot claim for i2c_davinci.1
> > > pinctrl-single 1c14120.pinmux: pin-4 (i2c_davinci.1) status -22
> > > pinctrl-single 1c14120.pinmux: could not request pin 4 on device pinctrl-single
> > > console [netcon0] enabled
> > > ....
> > > 
> > > This is because the mdio and i2c are using same pin 0x10,
> > 
> > How can two devices use the same pin? I mean physically, in hardware?
> > 
> > Is this because pinctrl-single uses the register address as the pin
> > number, whereas you have registers which configure multiple pins at
> > once? If so, your hardware isn't something that can be represented by
> > pinctrl-single.
> > 
> > > Is there any
> > > alternative way to handle if the two node's are using same pins any
> > > pointers could be very much helpful ?
> 
> It could also that the mux register(s) follow the one-mux-per-bit
> mapping.
> 
> In that case pinctrl-single,bits option as documented in the
> Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt.

Oh it's already using pinctrl-single,bits option. Maybe there's a
bug, adding Peter to cc.

Regards,

Tony

Re: Query on pinctrl usage for DT nodes

[Peter Ujfalusi]

Hi,

On 04/03/2013 08:42 PM, Tony Lindgren wrote:
> Oh it's already using pinctrl-single,bits option. Maybe there's a
> bug, adding Peter to cc.

I think all boils down to the fact that in pinctrl (single only?) _one_
register can be only associated with _one_ driver. Even through ,bit type
would allow us to only change parts of the same register, another device can
not take the same register with different mask.
I did noticed this after I have sent the bits-per-mux support.
Right now I don't have solution for this...

-- 
Péter

Re: Query on pinctrl usage for DT nodes

[Stephen Warren]

On 04/15/2013 02:19 AM, Peter Ujfalusi wrote:
> Hi,
> 
> On 04/03/2013 08:42 PM, Tony Lindgren wrote:
>> Oh it's already using pinctrl-single,bits option. Maybe there's a
>> bug, adding Peter to cc.
> 
> I think all boils down to the fact that in pinctrl (single only?) _one_
> register can be only associated with _one_ driver. Even through ,bit type
> would allow us to only change parts of the same register, another device can
> not take the same register with different mask.

pinctrl itself certainly imposes no such restriction that I'm aware of.
pinctrl is only concerned with the ownership of *pins* (or perhaps
groups). The mapping of pins to registers is something entirely internal
to individual pinctrl drivers.

So, the solution here is for pinctrl-single to create a separate
pin/group object for each separately controllable entity, and that may
be each bit in a register rather than each register, depending on HW.

Re: Query on pinctrl usage for DT nodes

[Peter Ujfalusi]

On 04/15/2013 06:40 PM, Stephen Warren wrote:
> So, the solution here is for pinctrl-single to create a separate
> pin/group object for each separately controllable entity, and that may
> be each bit in a register rather than each register, depending on HW.

Exactly what I was thinking.
It would be ideal if we could do this dynamically, so when a new
pinctr-single,bits is added we create a new object (after checking for
overlapping bits from previous objects).
If this is not possible we should have new property telling pinctrl-single how
many objects to create for one physical register (used only when
pinctrl-single,bit-per-mux is defined), something like:

pinctrl-single,bit-per-mux;
pinctrl-single,functions-per-register = <X>;

For sure we need to check here as well for overlaps in the bitfields...

-- 
Péter

Re: Query on pinctrl usage for DT nodes

[Prabhakar Lad]

Hi Stephen,

Sorry for the delayed response, I was a holiday.

On Wed, Apr 3, 2013 at 10:14 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 04/03/2013 03:16 AM, Prabhakar Lad wrote:
>> Hi Linus/Stephen,
>>
>> I am working adding  DT nodes for DA850.
>>
>> Following is the pin control snippet of da850.dtsi:-
>>
>>               pmx_core: pinmux@1c14120 {
> ...
>>                         i2c0_pins: pinmux_i2c0_pins {
>>                                 pinctrl-single,bits = <
>>                                         /* I2C0_SDA,I2C0_SCL */
>>                                         0x10 0x00002200 0x0000ff00
>>                                 >;
>>                         };
>>                         mdio_pins: pinmux_mdio_pins {
>>                                 pinctrl-single,bits = <
>>                                         /* MDIO_CLK, MDIO_D */
>>                                         0x10 0x00000088 0x000000ff
>>                                 >;
>>                         };
>>                 }
>>
>> And following is the code snippet for dts:-
>>
>>               i2c0: i2c@1c22000 {
>>                       status = "okay";
>>                       clock-frequency = <100000>;
>>                       pinctrl-names = "default";
>>                       pinctrl-0 = <&i2c0_pins>;
>>               };
>>               mdio: mdio@1e24000 {
>>                       status = "okay";
>>                       bus_freq = <2200000>;
>>                       pinctrl-names = "default";
>
> I assume there's also the following there:
>
>                         pinctrl-0 = <&mdio_pins>;
>
Yes missed it :)

>>               };
>>
>> But while booting I see the following boot log:-
>> ...
>> cpuidle: using governor menu
>> TCP: cubic registered
>> NET: Registered protocol family 17
>> pinctrl-single 1c14120.pinmux: pin 1c14130 already requested by
>> davinci_mdio.0; cannot claim for i2c_davinci.1
>> pinctrl-single 1c14120.pinmux: pin-4 (i2c_davinci.1) status -22
>> pinctrl-single 1c14120.pinmux: could not request pin 4 on device pinctrl-single
>> console [netcon0] enabled
>> ....
>>
>> This is because the mdio and i2c are using same pin 0x10,
>
> How can two devices use the same pin? I mean physically, in hardware?
>
> Is this because pinctrl-single uses the register address as the pin
> number, whereas you have registers which configure multiple pins at

Yes you are correct, we have registers  which configure multiple pins at once.
For example for above Pin Multiplexing Control 4 Register (Pinmux4)
[1] page(250) : -

PINMUX4_31_28 --> SP1_SCS[2]/UART1_TXD/SATA_CP_POD/GP1[0] Control
PINMUX4_27_24 --> SPI1_SCS[3]/UART1_RXD/SATA_LED/GP1[1] Control
PINMUX4_23_20 --> SPI1_SCS[4]/UART2_TXD/I2C1_SDA/GP1[2] Control
PINMUX4_19_16 --> SPI1_SCS[5]/UART2_RXD/I2C1_SCL/GP1[3] Control
PINMUX4_15_12 --> SPI1_SCS[6]/I2C0_SDA/TM64P3_OUT12/GP1[4] Control
PINMUX4_11_8 --> SPI1_SCS[7]/I2C0_SCL/TM64P2_OUT12/GP1[5] Control
PINMUX4_7_4 --> SPI0_SCS[0]/TM64P1_OUT12/GP1[6]/MDIO_D/TM64P1_IN12 Control
PINMUX4_3_0 --> SPI0_SCS[1]/TM64P0_OUT12/GP1[7]/MDIO_CLK/TM64P0_IN12 Control

> once? If so, your hardware isn't something that can be represented by
> pinctrl-single.
>
What is the alternative for such case ? any pointer would be helpful.

[1] http://www.ti.com/general/docs/lit/getliterature.tsp?literatureNumber=spruh77a&fileType=pdf

Regards,
--Prabhakar

Re: Query on pinctrl usage for DT nodes

[Stephen Warren]

On 04/08/2013 07:12 AM, Prabhakar Lad wrote:
> On Wed, Apr 3, 2013 at 10:14 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>> On 04/03/2013 03:16 AM, Prabhakar Lad wrote:
>>> Hi Linus/Stephen,
>>>
>>> I am working adding  DT nodes for DA850.
...
>>> But while booting I see the following boot log:-
>>> ...
>>> cpuidle: using governor menu
>>> TCP: cubic registered
>>> NET: Registered protocol family 17
>>> pinctrl-single 1c14120.pinmux: pin 1c14130 already requested by
>>> davinci_mdio.0; cannot claim for i2c_davinci.1
>>> pinctrl-single 1c14120.pinmux: pin-4 (i2c_davinci.1) status -22
>>> pinctrl-single 1c14120.pinmux: could not request pin 4 on device pinctrl-single
>>> console [netcon0] enabled
>>> ....
>>>
>>> This is because the mdio and i2c are using same pin 0x10,
>>
>> How can two devices use the same pin? I mean physically, in hardware?
>>
>> Is this because pinctrl-single uses the register address as the pin
>> number, whereas you have registers which configure multiple pins at
> 
> Yes you are correct, we have registers  which configure multiple pins at once.
> For example for above Pin Multiplexing Control 4 Register (Pinmux4)
> [1] page(250) : -
> 
> PINMUX4_31_28 --> SP1_SCS[2]/UART1_TXD/SATA_CP_POD/GP1[0] Control
> PINMUX4_27_24 --> SPI1_SCS[3]/UART1_RXD/SATA_LED/GP1[1] Control
> PINMUX4_23_20 --> SPI1_SCS[4]/UART2_TXD/I2C1_SDA/GP1[2] Control
> PINMUX4_19_16 --> SPI1_SCS[5]/UART2_RXD/I2C1_SCL/GP1[3] Control
> PINMUX4_15_12 --> SPI1_SCS[6]/I2C0_SDA/TM64P3_OUT12/GP1[4] Control
> PINMUX4_11_8 --> SPI1_SCS[7]/I2C0_SCL/TM64P2_OUT12/GP1[5] Control
> PINMUX4_7_4 --> SPI0_SCS[0]/TM64P1_OUT12/GP1[6]/MDIO_D/TM64P1_IN12 Control
> PINMUX4_3_0 --> SPI0_SCS[1]/TM64P0_OUT12/GP1[7]/MDIO_CLK/TM64P0_IN12 Control
> 
>> once? If so, your hardware isn't something that can be represented by
>> pinctrl-single.
>
> What is the alternative for such case ? any pointer would be helpful.
> 
> [1] http://www.ti.com/general/docs/lit/getliterature.tsp?literatureNumber=spruh77a&fileType=pdf

Tony previously replied:

Tony wrote:
> Tony wrote:
>> ??? wrote:
>>> Prabhakar wrote:
>>>> Is there any
>>>> alternative way to handle if the two node's are using same pins any
>>>> pointers could be very much helpful ?
>>
>> It could also that the mux register(s) follow the one-mux-per-bit
>> mapping.
>>
>> In that case pinctrl-single,bits option as documented in the
>> Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt.
> 
> Oh it's already using pinctrl-single,bits option. Maybe there's a
> bug, adding Peter to cc.

So I guess you need to debug the code to see why that option isn't
working for you.

Going back to your original question above though, if the pinctrl-single
driver doesn't work out for you, then you'd need to create a custom
pinctrl driver for your chip, just like the many examples already in
drivers/pinctrl.

Re: Query on pinctrl usage for DT nodes

[Prabhakar Lad]

Hi Stephen,Peter,

On Mon, Apr 8, 2013 at 10:54 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/08/2013 07:12 AM, Prabhakar Lad wrote:
>> On Wed, Apr 3, 2013 at 10:14 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/03/2013 03:16 AM, Prabhakar Lad wrote:
>>>> Hi Linus/Stephen,
>>>>
>>>> I am working adding  DT nodes for DA850.
> ...
>>>> But while booting I see the following boot log:-
>>>> ...
>>>> cpuidle: using governor menu
>>>> TCP: cubic registered
>>>> NET: Registered protocol family 17
>>>> pinctrl-single 1c14120.pinmux: pin 1c14130 already requested by
>>>> davinci_mdio.0; cannot claim for i2c_davinci.1
>>>> pinctrl-single 1c14120.pinmux: pin-4 (i2c_davinci.1) status -22
>>>> pinctrl-single 1c14120.pinmux: could not request pin 4 on device pinctrl-single
>>>> console [netcon0] enabled
>>>> ....
>>>>
>>>> This is because the mdio and i2c are using same pin 0x10,
>>>
>>> How can two devices use the same pin? I mean physically, in hardware?
>>>
>>> Is this because pinctrl-single uses the register address as the pin
>>> number, whereas you have registers which configure multiple pins at
>>
>> Yes you are correct, we have registers  which configure multiple pins at once.
>> For example for above Pin Multiplexing Control 4 Register (Pinmux4)
>> [1] page(250) : -
>>
>> PINMUX4_31_28 --> SP1_SCS[2]/UART1_TXD/SATA_CP_POD/GP1[0] Control
>> PINMUX4_27_24 --> SPI1_SCS[3]/UART1_RXD/SATA_LED/GP1[1] Control
>> PINMUX4_23_20 --> SPI1_SCS[4]/UART2_TXD/I2C1_SDA/GP1[2] Control
>> PINMUX4_19_16 --> SPI1_SCS[5]/UART2_RXD/I2C1_SCL/GP1[3] Control
>> PINMUX4_15_12 --> SPI1_SCS[6]/I2C0_SDA/TM64P3_OUT12/GP1[4] Control
>> PINMUX4_11_8 --> SPI1_SCS[7]/I2C0_SCL/TM64P2_OUT12/GP1[5] Control
>> PINMUX4_7_4 --> SPI0_SCS[0]/TM64P1_OUT12/GP1[6]/MDIO_D/TM64P1_IN12 Control
>> PINMUX4_3_0 --> SPI0_SCS[1]/TM64P0_OUT12/GP1[7]/MDIO_CLK/TM64P0_IN12 Control
>>
>>> once? If so, your hardware isn't something that can be represented by
>>> pinctrl-single.
>>
>> What is the alternative for such case ? any pointer would be helpful.
>>
>> [1] http://www.ti.com/general/docs/lit/getliterature.tsp?literatureNumber=spruh77a&fileType=pdf
>
> Tony previously replied:
>
> Tony wrote:
>> Tony wrote:
>>> ??? wrote:
>>>> Prabhakar wrote:
>>>>> Is there any
>>>>> alternative way to handle if the two node's are using same pins any
>>>>> pointers could be very much helpful ?
>>>
>>> It could also that the mux register(s) follow the one-mux-per-bit
>>> mapping.
>>>
>>> In that case pinctrl-single,bits option as documented in the
>>> Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt.
>>
>> Oh it's already using pinctrl-single,bits option. Maybe there's a
>> bug, adding Peter to cc.
>
> So I guess you need to debug the code to see why that option isn't
> working for you.
>
Following is the proposed fix/hack let me know if its OK.

Regards,
--Prabhakar

->>>>>>>>>>>>>>>>>>>

diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index ee72f1f..78fb42d 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -46,6 +46,7 @@ struct pinctrl_dev {
        struct pinctrl *p;
        struct pinctrl_state *hog_default;
        struct pinctrl_state *hog_sleep;
+       bool bits_per_mux;
 #ifdef CONFIG_DEBUG_FS
        struct dentry *device_root;
 #endif
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 5c32e88..d2a91ec 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -974,6 +974,7 @@ static int pcs_probe(struct platform_device *pdev)
                ret = -EINVAL;
                goto free;
        }
+       pcs->pctl->bits_per_mux = pcs->bits_per_mux;

        dev_info(pcs->dev, "%i pins at pa %p size %u\n",
                 pcs->desc.npins, pcs->base, pcs->size);
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 1a00658..4989f01 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -110,7 +110,7 @@ static int pin_request(struct pinctrl_dev *pctldev,

                desc->gpio_owner = owner;
        } else {
-               if (desc->mux_usecount && strcmp(desc->mux_owner, owner)) {
+               if (!pctldev->bits_per_mux && desc->mux_usecount &&
strcmp(desc->mux_owner, owner)) {
                        dev_err(pctldev->dev,
                                "pin %s already requested by %s;
cannot claim for %s\n",
                                desc->name, desc->mux_owner, owner);
@@ -118,7 +118,7 @@ static int pin_request(struct pinctrl_dev *pctldev,
                }

                desc->mux_usecount++;
-               if (desc->mux_usecount > 1)
+               if (desc->mux_usecount > 1 && !pctldev->bits_per_mux)
                        return 0;

                desc->mux_owner = owner;

Re: Query on pinctrl usage for DT nodes

[Stephen Warren]

On 04/10/2013 02:12 AM, Prabhakar Lad wrote:
...
> Following is the proposed fix/hack let me know if its OK.
> 
> Regards,
> --Prabhakar
> 
> ->>>>>>>>>>>>>>>>>>>
> 
> diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
> index ee72f1f..78fb42d 100644
> --- a/drivers/pinctrl/core.h
> +++ b/drivers/pinctrl/core.h
> @@ -46,6 +46,7 @@ struct pinctrl_dev {
>         struct pinctrl *p;
>         struct pinctrl_state *hog_default;
>         struct pinctrl_state *hog_sleep;
> +       bool bits_per_mux;

This clearly isn't correct; any change to solve this problem should only
touch the internals of the pinctrl-single driver, not the pinctrl core.

Re: Query on pinctrl usage for DT nodes

[Tony Lindgren]

* Stephen Warren <swarren@wwwdotorg.org> [130410 10:37]:
> On 04/10/2013 02:12 AM, Prabhakar Lad wrote:
> ...
> > Following is the proposed fix/hack let me know if its OK.
> > 
> > Regards,
> > --Prabhakar
> > 
> > ->>>>>>>>>>>>>>>>>>>
> > 
> > diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
> > index ee72f1f..78fb42d 100644
> > --- a/drivers/pinctrl/core.h
> > +++ b/drivers/pinctrl/core.h
> > @@ -46,6 +46,7 @@ struct pinctrl_dev {
> >         struct pinctrl *p;
> >         struct pinctrl_state *hog_default;
> >         struct pinctrl_state *hog_sleep;
> > +       bool bits_per_mux;
> 
> This clearly isn't correct; any change to solve this problem should only
> touch the internals of the pinctrl-single driver, not the pinctrl core.

Yeah how about just change the pintctrl-single,bits register
naming to be register + bit?  Something like 0xdeadbeef.0 and
0xdeadbeef.1 and so on.

Regards,

Tony

Re: Query on pinctrl usage for DT nodes

[Prabhakar Lad]

Hi Tony,

On Thu, Apr 11, 2013 at 2:04 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Stephen Warren <swarren@wwwdotorg.org> [130410 10:37]:
>> On 04/10/2013 02:12 AM, Prabhakar Lad wrote:
>> ...
>> > Following is the proposed fix/hack let me know if its OK.
>> >
>> > Regards,
>> > --Prabhakar
>> >
>> > ->>>>>>>>>>>>>>>>>>>
>> >
>> > diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
>> > index ee72f1f..78fb42d 100644
>> > --- a/drivers/pinctrl/core.h
>> > +++ b/drivers/pinctrl/core.h
>> > @@ -46,6 +46,7 @@ struct pinctrl_dev {
>> >         struct pinctrl *p;
>> >         struct pinctrl_state *hog_default;
>> >         struct pinctrl_state *hog_sleep;
>> > +       bool bits_per_mux;
>>
>> This clearly isn't correct; any change to solve this problem should only
>> touch the internals of the pinctrl-single driver, not the pinctrl core.
>
> Yeah how about just change the pintctrl-single,bits register
> naming to be register + bit?  Something like 0xdeadbeef.0 and
> 0xdeadbeef.1 and so on.
>
How and where the changing the register naming would help ?

Regards,
--Prabhakar

> Regards,
>
> Tony

Re: Query on pinctrl usage for DT nodes

[Prabhakar Lad]

Hi Tony,

On Mon, Apr 15, 2013 at 10:39 AM, Prabhakar Lad
<prabhakar.csengg@gmail.com> wrote:
> Hi Tony,
>
> On Thu, Apr 11, 2013 at 2:04 AM, Tony Lindgren <tony@atomide.com> wrote:
>> * Stephen Warren <swarren@wwwdotorg.org> [130410 10:37]:
>>> On 04/10/2013 02:12 AM, Prabhakar Lad wrote:
>>> ...
>>> > Following is the proposed fix/hack let me know if its OK.
>>> >
>>> > Regards,
>>> > --Prabhakar
>>> >
>>> > ->>>>>>>>>>>>>>>>>>>
>>> >
>>> > diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
>>> > index ee72f1f..78fb42d 100644
>>> > --- a/drivers/pinctrl/core.h
>>> > +++ b/drivers/pinctrl/core.h
>>> > @@ -46,6 +46,7 @@ struct pinctrl_dev {
>>> >         struct pinctrl *p;
>>> >         struct pinctrl_state *hog_default;
>>> >         struct pinctrl_state *hog_sleep;
>>> > +       bool bits_per_mux;
>>>
>>> This clearly isn't correct; any change to solve this problem should only
>>> touch the internals of the pinctrl-single driver, not the pinctrl core.
>>
>> Yeah how about just change the pintctrl-single,bits register
>> naming to be register + bit?  Something like 0xdeadbeef.0 and
>> 0xdeadbeef.1 and so on.
>>
> How and where the changing the register naming would help ?
>
No matter we change the register name, it would fail in pin_request()
function, see the following code snippet

static int pin_request(struct pinctrl_dev *pctldev,
		       int pin, const char *owner,
		       struct pinctrl_gpio_range *gpio_range)
{
	struct pin_desc *desc;
	const struct pinmux_ops *ops = pctldev->desc->pmxops;
	int status = -EINVAL;

	desc = pin_desc_get(pctldev, pin);
	if (desc == NULL) {
		dev_err(pctldev->dev,
			"pin %d is not registered so it cannot be requested\n",
			pin);
		goto out;
	}

	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
		pin, desc->name, owner);

	if (gpio_range) {
		/* There's no need to support multiple GPIO requests */
		if (desc->gpio_owner) {
			dev_err(pctldev->dev,
				"pin %s already requested by %s; cannot claim for %s\n",
				desc->name, desc->gpio_owner, owner);
			goto out;
		}

		desc->gpio_owner = owner;
	} else {
		if (desc->mux_usecount && strcmp(desc->mux_owner, owner)) {
			dev_err(pctldev->dev,
				"pin %s already requested by %s; cannot claim for %s\n",
				desc->name, desc->mux_owner, owner);
			goto out;
		}

		desc->mux_usecount++;
		if (desc->mux_usecount > 1)
			return 0;

		desc->mux_owner = owner;
	}

      ...........
}

Since desc->mux_owner and owner would obviously differ.

Regards,
--Prabhakar

> Regards,
> --Prabhakar
>
>> Regards,
>>
>> Tony

Re: Query on pinctrl usage for DT nodes

[Peter Ujfalusi]

On 04/10/2013 10:34 PM, Tony Lindgren wrote:
> Yeah how about just change the pintctrl-single,bits register
> naming to be register + bit?  Something like 0xdeadbeef.0 and
> 0xdeadbeef.1 and so on.

Something like this might work I think. It is going to be a bit tricky IMHO
since we might need span out new 'register' every time a device requests for a
new pinctrl-single,bits for already used register in the
pinctrl-single,bit-per-mux area. In this way we still can make sure that
certain bit are only used by a single driver.

-- 
Péter

Re: Query on pinctrl usage for DT nodes

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org> [130415 01:30]:
> On 04/10/2013 10:34 PM, Tony Lindgren wrote:
> > Yeah how about just change the pintctrl-single,bits register
> > naming to be register + bit?  Something like 0xdeadbeef.0 and
> > 0xdeadbeef.1 and so on.
> 
> Something like this might work I think. It is going to be a bit tricky IMHO
> since we might need span out new 'register' every time a device requests for a
> new pinctrl-single,bits for already used register in the
> pinctrl-single,bit-per-mux area. In this way we still can make sure that
> certain bit are only used by a single driver.

OK. If it's one bit per mux type register we should be able to create
the right amount of entries based on the submask in pinctrs-single,bit?

Regards,

Tony

Re: Query on pinctrl usage for DT nodes

[Peter Ujfalusi]

On 04/16/2013 11:32 PM, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [130415 01:30]:
>> On 04/10/2013 10:34 PM, Tony Lindgren wrote:
>>> Yeah how about just change the pintctrl-single,bits register
>>> naming to be register + bit?  Something like 0xdeadbeef.0 and
>>> 0xdeadbeef.1 and so on.
>>
>> Something like this might work I think. It is going to be a bit tricky IMHO
>> since we might need span out new 'register' every time a device requests for a
>> new pinctrl-single,bits for already used register in the
>> pinctrl-single,bit-per-mux area. In this way we still can make sure that
>> certain bit are only used by a single driver.
> 
> OK. If it's one bit per mux type register we should be able to create
> the right amount of entries based on the submask in pinctrs-single,bit?

Right now it seams to be true that we have one bit per mux (in DEVCONF0 on
OMAP3 for example). So that would work fine, but There could be different
registers around with more than one bit per mux.

Another way to deal with this is to:
in case of pinctrl-single,bit-per-mux we assume one bit per mux and create
entries based on the pinctrl-single,function-mask's bits.

In case we have more than one bit for the mux in the register we could have
optional property stating the number of different muxes handled by the register.

One bit per mux type:

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>;
};

Results six entries.

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,functions-in-register = <3>;
        pinctrl-single,register-width = <32>;
        pinctrl-single,function-mask = <0x5F>;
};

Will results three entries.

In both cases we still need to test overlaps in the handled bits.

-- 
Péter

Re: Query on pinctrl usage for DT nodes

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org> [130423 00:47]:
> On 04/16/2013 11:32 PM, Tony Lindgren wrote:
> > * Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org> [130415 01:30]:
> >> On 04/10/2013 10:34 PM, Tony Lindgren wrote:
> >>> Yeah how about just change the pintctrl-single,bits register
> >>> naming to be register + bit?  Something like 0xdeadbeef.0 and
> >>> 0xdeadbeef.1 and so on.
> >>
> >> Something like this might work I think. It is going to be a bit tricky IMHO
> >> since we might need span out new 'register' every time a device requests for a
> >> new pinctrl-single,bits for already used register in the
> >> pinctrl-single,bit-per-mux area. In this way we still can make sure that
> >> certain bit are only used by a single driver.
> > 
> > OK. If it's one bit per mux type register we should be able to create
> > the right amount of entries based on the submask in pinctrs-single,bit?
> 
> Right now it seams to be true that we have one bit per mux (in DEVCONF0 on
> OMAP3 for example). So that would work fine, but There could be different
> registers around with more than one bit per mux.

Yes you are right, we should cover that case too. But maybe we
can wait until we have such an example :)
 
> Another way to deal with this is to:
> in case of pinctrl-single,bit-per-mux we assume one bit per mux and create
> entries based on the pinctrl-single,function-mask's bits.
> 
> In case we have more than one bit for the mux in the register we could have
> optional property stating the number of different muxes handled by the register.

Yes that's doable.
 
> One bit per mux type:
> 
> 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>;
> };
> 
> Results six entries.

Yup, I think this is the way to go for now, see below..
 
> 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,functions-in-register = <3>;
>         pinctrl-single,register-width = <32>;
>         pinctrl-single,function-mask = <0x5F>;
> };
> 
> Will results three entries.

..but let's not add this. The reason why I'd like to postpone adding
pinctrl-single,functions-in-register is because we may be able to
do it automatically.

If it was just the naming is the issue, we could use the submask
for the naming the bits with something like 0x0x48002274-3-1 and
0x0x48002274-1-1.

But I think currently the pinctrl framework expects a static pin
table, so we can't create these entries dynamically when the
consumer driver starts using the bits. So the right fix there
would be to improve the pinctrl framework to allow adding pins
dynamically, see some of the REVISIT comments in pinctrl-single.c.

> In both cases we still need to test overlaps in the handled bits.

Yes that's missing too.

So how about we do the following for now:

1. Fix the pinctrl-single,bits naming to use 0x0x48002274-1-1 to
   represent the bit range

2. Automatically create entries for each bit and don't yet support
   bit ranges except in the naming for future use

3. Add something to check the overlaps in the submasks

4. Assume we have one-bit-per-mux until we have a real example,
   then improve the pinctrl framework to allow adding pins 
   dynamically as the consumer driver probes. Or if there are
   other issues, add the pinctrl-single,functions-in-register
   property at that point.

Regards,

Tony

RE: Query on pinctrl usage for DT nodes

[Manjunathappa, Prakash]

Hi,

On Tue, Apr 23, 2013 at 23:47:54, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org> [130423 00:47]:
> > On 04/16/2013 11:32 PM, Tony Lindgren wrote:
> > > * Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org> [130415 01:30]:
> > >> On 04/10/2013 10:34 PM, Tony Lindgren wrote:
> > >>> Yeah how about just change the pintctrl-single,bits register
> > >>> naming to be register + bit?  Something like 0xdeadbeef.0 and
> > >>> 0xdeadbeef.1 and so on.
> > >>
> > >> Something like this might work I think. It is going to be a bit tricky IMHO
> > >> since we might need span out new 'register' every time a device requests for a
> > >> new pinctrl-single,bits for already used register in the
> > >> pinctrl-single,bit-per-mux area. In this way we still can make sure that
> > >> certain bit are only used by a single driver.
> > > 
> > > OK. If it's one bit per mux type register we should be able to create
> > > the right amount of entries based on the submask in pinctrs-single,bit?
> > 
> > Right now it seams to be true that we have one bit per mux (in DEVCONF0 on
> > OMAP3 for example). So that would work fine, but There could be different
> > registers around with more than one bit per mux.
> 
> Yes you are right, we should cover that case too. But maybe we
> can wait until we have such an example :)
>  
> > Another way to deal with this is to:
> > in case of pinctrl-single,bit-per-mux we assume one bit per mux and create
> > entries based on the pinctrl-single,function-mask's bits.
> > 
> > In case we have more than one bit for the mux in the register we could have
> > optional property stating the number of different muxes handled by the register.
> 
> Yes that's doable.
>  
> > One bit per mux type:
> > 
> > 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>;
> > };
> > 
> > Results six entries.
> 
> Yup, I think this is the way to go for now, see below..
>  
> > 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,functions-in-register = <3>;
> >         pinctrl-single,register-width = <32>;
> >         pinctrl-single,function-mask = <0x5F>;
> > };
> > 
> > Will results three entries.
> 
> ..but let's not add this. The reason why I'd like to postpone adding
> pinctrl-single,functions-in-register is because we may be able to
> do it automatically.
> 

I have posted patch to take care of this:
http://davinci-linux-open-source.1494791.n2.nabble.com/PATCH-0-3-pinctrl-pinctrl-single-Add-full-fledge-support-to-configure-multiple-pins-of-different-mods-tt7583066.html

Please review this solution.

Thanks,
Prakash