| Message ID | 1482832070-22668-6-git-send-email-ping.bai@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 27, 2016 at 10:47 AM, Bai Ping <ping.bai@nxp.com> wrote: > Add pinctrl binding doc update for imx6sll. > > Signed-off-by: Bai Ping <ping.bai@nxp.com> I have to push back on this a bit. > +Please refer to fsl,imx-pinctrl.txt in this directory for common binding part > +and usage. I understand that it is building on top of the old i.MX bindings and that it has some kind of "tradition" coming with it. At the same time, the i.MX bindings came about before we had the generic pin control bindings defined. > +CONFIG bits definition: > +PAD_CTL_LVE (1 << 22) > +PAD_CTL_HYS (1 << 16) > +PAD_CTL_PUS_100K_DOWN (0 << 14) > +PAD_CTL_PUS_47K_UP (1 << 14) > +PAD_CTL_PUS_100K_UP (2 << 14) > +PAD_CTL_PUS_22K_UP (3 << 14) > +PAD_CTL_PUE (1 << 13) > +PAD_CTL_PKE (1 << 12) > +PAD_CTL_ODE (1 << 11) > +PAD_CTL_SPEED_LOW (0 << 6) > +PAD_CTL_SPEED_MED (1 << 6) > +PAD_CTL_SPEED_HIGH (3 << 6) > +PAD_CTL_DSE_DISABLE (0 << 3) > +PAD_CTL_DSE_260ohm (1 << 3) > +PAD_CTL_DSE_130ohm (2 << 3) > +PAD_CTL_DSE_87ohm (3 << 3) > +PAD_CTL_DSE_65ohm (4 << 3) > +PAD_CTL_DSE_52ohm (5 << 3) > +PAD_CTL_DSE_43ohm (6 << 3) > +PAD_CTL_DSE_37ohm (7 << 3) > +PAD_CTL_SRE_FAST (1 << 0) > +PAD_CTL_SRE_SLOW (0 << 0) A whole slew of these if not all correspond to the generic bindings. I would consider augmenting the code in the driver to handle the generic bindings *in addition* to the old legacy bindings, and use those over these random custom bits. Read drivers using CONFIG_GENERIC_PINCONF as an inspiration. For example see commit cefbf1a1b29531a970bc2908a50a75d6474fcc38 "pinctrl: sunxi: Support generic binding" from Maxime Ripard, where he does a similar thing for sunxi. Yours, Linus Walleij
> Subject: Re: [PATCH v2 05/12] Document: dt: binding: imx: update pinctrl doc > for imx6sll > > On Tue, Dec 27, 2016 at 10:47 AM, Bai Ping <ping.bai@nxp.com> wrote: > > > Add pinctrl binding doc update for imx6sll. > > > > Signed-off-by: Bai Ping <ping.bai@nxp.com> > > I have to push back on this a bit. > > > +Please refer to fsl,imx-pinctrl.txt in this directory for common > > +binding part and usage. > > I understand that it is building on top of the old i.MX bindings and that it has > some kind of "tradition" coming with it. > > At the same time, the i.MX bindings came about before we had the generic pin > control bindings defined. > > > +CONFIG bits definition: > > +PAD_CTL_LVE (1 << 22) > > +PAD_CTL_HYS (1 << 16) > > +PAD_CTL_PUS_100K_DOWN (0 << 14) > > +PAD_CTL_PUS_47K_UP (1 << 14) > > +PAD_CTL_PUS_100K_UP (2 << 14) > > +PAD_CTL_PUS_22K_UP (3 << 14) > > +PAD_CTL_PUE (1 << 13) > > +PAD_CTL_PKE (1 << 12) > > +PAD_CTL_ODE (1 << 11) > > +PAD_CTL_SPEED_LOW (0 << 6) > > +PAD_CTL_SPEED_MED (1 << 6) > > +PAD_CTL_SPEED_HIGH (3 << 6) > > +PAD_CTL_DSE_DISABLE (0 << 3) > > +PAD_CTL_DSE_260ohm (1 << 3) > > +PAD_CTL_DSE_130ohm (2 << 3) > > +PAD_CTL_DSE_87ohm (3 << 3) > > +PAD_CTL_DSE_65ohm (4 << 3) > > +PAD_CTL_DSE_52ohm (5 << 3) > > +PAD_CTL_DSE_43ohm (6 << 3) > > +PAD_CTL_DSE_37ohm (7 << 3) > > +PAD_CTL_SRE_FAST (1 << 0) > > +PAD_CTL_SRE_SLOW (0 << 0) > > A whole slew of these if not all correspond to the generic bindings. > > I would consider augmenting the code in the driver to handle the generic > bindings *in addition* to the old legacy bindings, and use those over these > random custom bits. > > Read drivers using CONFIG_GENERIC_PINCONF as an inspiration. > > For example see commit > cefbf1a1b29531a970bc2908a50a75d6474fcc38 > "pinctrl: sunxi: Support generic binding" > from Maxime Ripard, where he does a similar thing for sunxi. I have look into the above commit on using generic binding. But I think the generic pinconf is not very easy to add in imx pinctrl Driver. imx pinctrl use a different way to parse the pin configure. Each fsl,pin entry looks like <PIN_FUNC_ID CONFIG> in dts, the CONFIG is the pad setting value like pull-up, open-drain, drive strength etc. The above config bit definition is specific to each SOC in the PAD CTL register. If we want set the pin config to enable hysteresis, 47KOhm Pull Up, 50Mhz speed, 80Ohm driver strength and Fast Slew Rate, then the CONFIG value should be 0x17059( ORs corresponding bit definition). This value will be set in PAD CTL register to config the corresponding pin. BR Jacky Bai > > Yours, > Linus Walleij
On Mon, Jan 9, 2017 at 3:32 AM, Jacky Bai <ping.bai@nxp.com> wrote: > I have look into the above commit on using generic binding. But I think the generic pinconf > is not very easy to add in imx pinctrl Driver. imx pinctrl use a different way to parse the pin configure. OK atleast I need an indication from one of the i.MX maintainers how they want to proceed. > Each fsl,pin entry looks like <PIN_FUNC_ID CONFIG> in dts, the CONFIG is the pad setting value like > pull-up, open-drain, drive strength etc. The above config bit definition is specific to each SOC in the PAD CTL register. > > If we want set the pin config to enable hysteresis, 47KOhm Pull Up, 50Mhz speed, 80Ohm driver strength > and Fast Slew Rate, then the CONFIG value should be 0x17059( ORs corresponding bit definition). Hysteresis is an input property and does not make sense on something that need drive strength and slew rate configuration which are output properties. (I guess you mean 80mA drive strength.) Actually such oxymoronic settings is a good reason to migrate to generic bindings because when you describe stuff with generic strings you see better what is going on, and we can add sanity checks to cases like this in the generic code where it would indeed be valid to ask why this combination of settings is being made. > This value will be set in > PAD CTL register to config the corresponding pin. Yes? That is common. It looks like that in DT: { input-schmitt-enable; bias-pull-up = <47000>; slew-rate = <50000000>; drive-strength = <80000>; }; Yours, Linus Walleij
> Subject: Re: [PATCH v2 05/12] Document: dt: binding: imx: update pinctrl doc > for imx6sll > > On Mon, Jan 9, 2017 at 3:32 AM, Jacky Bai <ping.bai@nxp.com> wrote: > > > I have look into the above commit on using generic binding. But I > > think the generic pinconf is not very easy to add in imx pinctrl Driver. imx > pinctrl use a different way to parse the pin configure. > > OK atleast I need an indication from one of the i.MX maintainers how they > want to proceed. > > > Each fsl,pin entry looks like <PIN_FUNC_ID CONFIG> in dts, the > > CONFIG is the pad setting value like pull-up, open-drain, drive strength etc. > The above config bit definition is specific to each SOC in the PAD CTL register. > > > > If we want set the pin config to enable hysteresis, 47KOhm Pull Up, > > 50Mhz speed, 80Ohm driver strength and Fast Slew Rate, then the CONFIG > value should be 0x17059( ORs corresponding bit definition). > > Hysteresis is an input property and does not make sense on something that > need drive strength and slew rate configuration which are output properties. > (I guess you mean 80mA drive strength.) > For some bi-direction pins, like SD DATA pin, we may need to enable the hysteresis configuration. > Actually such oxymoronic settings is a good reason to migrate to generic > bindings because when you describe stuff with generic strings you see better > what is going on, and we can add sanity checks to cases like this in the generic > code where it would indeed be valid to ask why this combination of settings is > being made. > Another thing is that we can use a pins-tool program developed by NXP to generate the pinctrl configuration code that can be used directly in dts. This tiny program can avoid pin function conflict. As on i.MX, there are so may pins, each pin can be used for up 8 function. Configuring the pins is a time-consuming work. This tools is very useful for customer to generate the dts code. Using generic pinconf is another option, but it may need more time to add it and more work to do. For now, I think we can still keep the legacy method? > > This value will be set in > > PAD CTL register to config the corresponding pin. > > Yes? That is common. It looks like that in DT: > > { > input-schmitt-enable; > bias-pull-up = <47000>; > slew-rate = <50000000>; > drive-strength = <80000>; > }; > > Yours, > Linus Walleij
On Thu, Jan 12, 2017 at 3:57 AM, Jacky Bai <ping.bai@nxp.com> wrote: > Another thing is that we can use a pins-tool program developed by NXP to > generate the pinctrl configuration code that can be used directly in dts. This > tiny program can avoid pin function conflict. As on i.MX, there are so may pins, > each pin can be used for up 8 function. Configuring the pins is a time-consuming > work. This tools is very useful for customer to generate the dts code. I understand, but every silicon vendor has such a tool, all are different, proprietary and unfriendly to programmers and open source developers, who need to understand how the hardware is working without magic tools and secret data sheets to fix bugs. For the people working with maintaining the code it is paramount that DTS files are self-descriptive. Yours, Linus Walleij
> Subject: Re: [PATCH v2 05/12] Document: dt: binding: imx: update pinctrl doc > for imx6sll > > On Thu, Jan 12, 2017 at 3:57 AM, Jacky Bai <ping.bai@nxp.com> wrote: > > > Another thing is that we can use a pins-tool program developed by NXP > > to generate the pinctrl configuration code that can be used directly > > in dts. This tiny program can avoid pin function conflict. As on i.MX, > > there are so may pins, each pin can be used for up 8 function. > > Configuring the pins is a time-consuming work. This tools is very useful for > customer to generate the dts code. > > I understand, but every silicon vendor has such a tool, all are different, > proprietary and unfriendly to programmers and open source developers, who > need to understand how the hardware is working without magic tools and > secret data sheets to fix bugs. > > For the people working with maintaining the code it is paramount that DTS files > are self-descriptive. > OK. Thanks for your comments. Adding generic-pinconf in imx pinctrl needs some time to finish and the legacy method still need be here even if generic-pinconf is added. Do you plan to pick this legacy binding patch for now? > Yours, > Linus Walleij
On Tue, Jan 17, 2017 at 7:35 AM, Jacky Bai <ping.bai@nxp.com> wrote: >> Subject: Re: [PATCH v2 05/12] Document: dt: binding: imx: update pinctrl doc >> On Thu, Jan 12, 2017 at 3:57 AM, Jacky Bai <ping.bai@nxp.com> wrote: >> >> > Another thing is that we can use a pins-tool program developed by NXP >> > to generate the pinctrl configuration code that can be used directly >> > in dts. This tiny program can avoid pin function conflict. As on i.MX, >> > there are so may pins, each pin can be used for up 8 function. >> > Configuring the pins is a time-consuming work. This tools is very useful for >> customer to generate the dts code. >> >> I understand, but every silicon vendor has such a tool, all are different, >> proprietary and unfriendly to programmers and open source developers, who >> need to understand how the hardware is working without magic tools and >> secret data sheets to fix bugs. >> >> For the people working with maintaining the code it is paramount that DTS files >> are self-descriptive. > > OK. Thanks for your comments. Adding generic-pinconf in imx pinctrl needs some time > to finish and the legacy method still need be here even if generic-pinconf is added. > Do you plan to pick this legacy binding patch for now? As I said earlier: > atleast I need an indication from one of the i.MX maintainers how they > want to proceed. Yours, Linus Walleij
On Wed, Jan 18, 2017 at 01:24:30PM +0100, Linus Walleij wrote: > On Tue, Jan 17, 2017 at 7:35 AM, Jacky Bai <ping.bai@nxp.com> wrote: > >> Subject: Re: [PATCH v2 05/12] Document: dt: binding: imx: update pinctrl doc > >> On Thu, Jan 12, 2017 at 3:57 AM, Jacky Bai <ping.bai@nxp.com> wrote: > >> > >> > Another thing is that we can use a pins-tool program developed by NXP > >> > to generate the pinctrl configuration code that can be used directly > >> > in dts. This tiny program can avoid pin function conflict. As on i.MX, > >> > there are so may pins, each pin can be used for up 8 function. > >> > Configuring the pins is a time-consuming work. This tools is very useful for > >> customer to generate the dts code. > >> > >> I understand, but every silicon vendor has such a tool, all are different, > >> proprietary and unfriendly to programmers and open source developers, who > >> need to understand how the hardware is working without magic tools and > >> secret data sheets to fix bugs. > >> > >> For the people working with maintaining the code it is paramount that DTS files > >> are self-descriptive. > > > > OK. Thanks for your comments. Adding generic-pinconf in imx pinctrl needs some time > > to finish and the legacy method still need be here even if generic-pinconf is added. > > Do you plan to pick this legacy binding patch for now? > > As I said earlier: > > > atleast I need an indication from one of the i.MX maintainers how they > > want to proceed. Sorry for being late for the discussion. To be honest, I did not really expect so many i.MX6 variants coming to us. Just out of curiosity, are there any more in the pipeline to come, or is this the last known one? I understand that there are now more generic support available at pinctrl core level for us to use than the time we initial designed i.MX pinctrl driver. But I intend to agree with Jacky that imx6sll can be supported by the existing driver, since it doesn't require any additional driver changes. If any new generation of i.MX family require changes on the existing pinctrl driver, we should probably push back for a redesign to use generic support as much as possible. @Jacky, how does i.MX8 pinctrl look like? How same or different as/from i.MX6 pinctrl? Just my opinion. Shawn
> From: Shawn Guo [mailto:shawnguo@kernel.org] > > >> > > >> > Another thing is that we can use a pins-tool program developed by > > >> > NXP to generate the pinctrl configuration code that can be used > > >> > directly in dts. This tiny program can avoid pin function > > >> > conflict. As on i.MX, there are so may pins, each pin can be used for up 8 > function. > > >> > Configuring the pins is a time-consuming work. This tools is > > >> > very useful for > > >> customer to generate the dts code. > > >> > > >> I understand, but every silicon vendor has such a tool, all are > > >> different, proprietary and unfriendly to programmers and open > > >> source developers, who need to understand how the hardware is > > >> working without magic tools and secret data sheets to fix bugs. > > >> > > >> For the people working with maintaining the code it is paramount > > >> that DTS files are self-descriptive. > > > > > > OK. Thanks for your comments. Adding generic-pinconf in imx > > > pinctrl needs some time to finish and the legacy method still need be here > even if generic-pinconf is added. > > > Do you plan to pick this legacy binding patch for now? > > > > As I said earlier: > > > > > atleast I need an indication from one of the i.MX maintainers how > > > they want to proceed. > > Sorry for being late for the discussion. > > To be honest, I did not really expect so many i.MX6 variants coming to us. Just > out of curiosity, are there any more in the pipeline to come, or is this the last > known one? > AFAIK, this should be the last variant of i.MX6. But not the last one of i.MX family. > I understand that there are now more generic support available at pinctrl core > level for us to use than the time we initial designed i.MX pinctrl driver. But I > intend to agree with Jacky that imx6sll can be supported by the existing driver, > since it doesn't require any additional driver changes. > > If any new generation of i.MX family require changes on the existing pinctrl > driver, we should probably push back for a redesign to use generic support as > much as possible. @Jacky, how does i.MX8 pinctrl look like? How same or > different as/from i.MX6 pinctrl? > i.MX8 kernel support is in development stage. The code has not been finalized. For now, i.MX8 shares most of the pinctrl code with i.MX6 and use the same pinconfig method. > Just my opinion. > > Shawn
On Mon, Jan 23, 2017 at 8:17 AM, Jacky Bai <ping.bai@nxp.com> wrote: > i.MX8 kernel support is in development stage. The code has not been finalized. > For now, i.MX8 shares most of the pinctrl code with i.MX6 and use the same pinconfig method. Please switch to generic bindings already. Yours, Linus Walleij
Hi Linus, On Tue, Dec 27, 2016 at 8:59 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Dec 27, 2016 at 10:47 AM, Bai Ping <ping.bai@nxp.com> wrote: > >> Add pinctrl binding doc update for imx6sll. >> >> Signed-off-by: Bai Ping <ping.bai@nxp.com> > > I have to push back on this a bit. > >> +Please refer to fsl,imx-pinctrl.txt in this directory for common binding part >> +and usage. > > I understand that it is building on top of the old i.MX bindings and that > it has some kind of "tradition" coming with it. > > At the same time, the i.MX bindings came about before we had the > generic pin control bindings defined. > >> +CONFIG bits definition: >> +PAD_CTL_LVE (1 << 22) >> +PAD_CTL_HYS (1 << 16) >> +PAD_CTL_PUS_100K_DOWN (0 << 14) >> +PAD_CTL_PUS_47K_UP (1 << 14) >> +PAD_CTL_PUS_100K_UP (2 << 14) >> +PAD_CTL_PUS_22K_UP (3 << 14) >> +PAD_CTL_PUE (1 << 13) >> +PAD_CTL_PKE (1 << 12) >> +PAD_CTL_ODE (1 << 11) >> +PAD_CTL_SPEED_LOW (0 << 6) >> +PAD_CTL_SPEED_MED (1 << 6) >> +PAD_CTL_SPEED_HIGH (3 << 6) >> +PAD_CTL_DSE_DISABLE (0 << 3) >> +PAD_CTL_DSE_260ohm (1 << 3) >> +PAD_CTL_DSE_130ohm (2 << 3) >> +PAD_CTL_DSE_87ohm (3 << 3) >> +PAD_CTL_DSE_65ohm (4 << 3) >> +PAD_CTL_DSE_52ohm (5 << 3) >> +PAD_CTL_DSE_43ohm (6 << 3) >> +PAD_CTL_DSE_37ohm (7 << 3) >> +PAD_CTL_SRE_FAST (1 << 0) >> +PAD_CTL_SRE_SLOW (0 << 0) > > A whole slew of these if not all correspond to the generic bindings. > > I would consider augmenting the code in the driver to handle the generic > bindings *in addition* to the old legacy bindings, and use those over these > random custom bits. > > Read drivers using CONFIG_GENERIC_PINCONF as an inspiration. > > For example see commit > cefbf1a1b29531a970bc2908a50a75d6474fcc38 > "pinctrl: sunxi: Support generic binding" > from Maxime Ripard, where he does a similar thing for sunxi. > First thanks for your good suggestion! All we realized that the raw data of IMX pad config in dts is not good. e.g. pinctrl_ecspi3: ecspi3grp { fsl,pins = < MX7D_PAD_SAI2_TX_SYNC__ECSPI3_MISO 0x17059 ......... I did a deep feasibility analysis these days on the migrating to generic pinctrl binding you referred. It is regrettably that it may not be so suitable for IMX to fully migrate right now. Generic binding only supports parsing strings for pins and function from dts right now, that means we need back to the old way of hardcoding all register bits in driver which we actually chose to move out since the commit "e164153 pinctrl: imx: move hard-coding data into device tree". And IMX doesn't have PAD GROUP in HW, each pad theoretically can be muxed as 8 single functions. e.g. For IOMUXC_SW_MUX_CTL_PAD_CSI_DATA07 MUX_MODE MUX Mode Select Field. Select 1 of 8 iomux modes to be used for pad: CSI_DATA07. 000 ALT0 — Select signal CSI1_DATA09. 001 ALT1 — Select signal ESAI_TX3_RX2. 010 ALT2 — Select signal I2C4_SDA. 011 ALT3 — Select signal KPP_ROW7. 100 ALT4 — Select signal UART6_CTS_B. 101 ALT5 — Select signal GPIO1_IO21. 110 ALT6 — Select signal EIM_DATA16. 111 ALT7 — Select signal DCIC1_OUT. By converting to generic binding, we need construct a huge function/group map in driver for hundreds of pads for each SoC. That is a bit fear to us. Current IMX method only generates the used function/groups dynamically by parsing board dts which is much a lighter way. But of course, we want to fix the raw config value issue as well which is un-maintainable and un-readable as you pointed out. I think there might be two options for us to go: One option is using macro definitions instead of raw data as pinctrl-single users as OMAP. e.g. art2_pins: pinmux_uart2_pins { pinctrl-single,pins = < OMAP4_IOPAD(0x118, PIN_INPUT_PULLUP | MUX_MODE0) /* uart2_cts.uart2_cts */ ........ For this way, no driver changes required, just replace the raw data by macro in device tree. But i wonder this may not be your original intention although it's the easiest way for us. Anyway, if you agree, we probably would prefer this way. Another option is partially convert to generic binding for only pad configuration part. e.g. pinctrl_usdhc1: usdhc1grp { fsl,pins = < MX7D_PAD_SD1_CMD__SD1_CMD MX7D_PAD_SD1_CLK__SD1_CLK MX7D_PAD_SD1_DATA0__SD1_DATA0 ... >; bias-pull-up = <47KOHM>; drive-strength = <130OHM>; slew-rate = <FAST>; speed = <50MHZ>; .... }; (Some of the property still not supported or not the same as generic binding right now) This way requires no big drivers changes and only extend the driver to support the generic pinctrl dt configuration properties. And it also fix the raw data issues and looks like more applicable than the full migration. So would you be willing to accept it? Another potential benefits for this way is that we may can use PCONFDUMP to dump a more readable data from pinctrl debug system. > Yours, > Linus Walleij > -- > To unsubscribe from this list: send the line "unsubscribe linux-clk" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Regards Dong Aisheng
Hi Linus Gently Ping... Any feedback about the proposal? Regards Dong Aisheng On Thu, Feb 16, 2017 at 3:51 PM, Dong Aisheng <dongas86@gmail.com> wrote: > Hi Linus, > > On Tue, Dec 27, 2016 at 8:59 PM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Tue, Dec 27, 2016 at 10:47 AM, Bai Ping <ping.bai@nxp.com> wrote: >> >>> Add pinctrl binding doc update for imx6sll. >>> >>> Signed-off-by: Bai Ping <ping.bai@nxp.com> >> >> I have to push back on this a bit. >> >>> +Please refer to fsl,imx-pinctrl.txt in this directory for common binding part >>> +and usage. >> >> I understand that it is building on top of the old i.MX bindings and that >> it has some kind of "tradition" coming with it. >> >> At the same time, the i.MX bindings came about before we had the >> generic pin control bindings defined. >> >>> +CONFIG bits definition: >>> +PAD_CTL_LVE (1 << 22) >>> +PAD_CTL_HYS (1 << 16) >>> +PAD_CTL_PUS_100K_DOWN (0 << 14) >>> +PAD_CTL_PUS_47K_UP (1 << 14) >>> +PAD_CTL_PUS_100K_UP (2 << 14) >>> +PAD_CTL_PUS_22K_UP (3 << 14) >>> +PAD_CTL_PUE (1 << 13) >>> +PAD_CTL_PKE (1 << 12) >>> +PAD_CTL_ODE (1 << 11) >>> +PAD_CTL_SPEED_LOW (0 << 6) >>> +PAD_CTL_SPEED_MED (1 << 6) >>> +PAD_CTL_SPEED_HIGH (3 << 6) >>> +PAD_CTL_DSE_DISABLE (0 << 3) >>> +PAD_CTL_DSE_260ohm (1 << 3) >>> +PAD_CTL_DSE_130ohm (2 << 3) >>> +PAD_CTL_DSE_87ohm (3 << 3) >>> +PAD_CTL_DSE_65ohm (4 << 3) >>> +PAD_CTL_DSE_52ohm (5 << 3) >>> +PAD_CTL_DSE_43ohm (6 << 3) >>> +PAD_CTL_DSE_37ohm (7 << 3) >>> +PAD_CTL_SRE_FAST (1 << 0) >>> +PAD_CTL_SRE_SLOW (0 << 0) >> >> A whole slew of these if not all correspond to the generic bindings. >> >> I would consider augmenting the code in the driver to handle the generic >> bindings *in addition* to the old legacy bindings, and use those over these >> random custom bits. >> >> Read drivers using CONFIG_GENERIC_PINCONF as an inspiration. >> >> For example see commit >> cefbf1a1b29531a970bc2908a50a75d6474fcc38 >> "pinctrl: sunxi: Support generic binding" >> from Maxime Ripard, where he does a similar thing for sunxi. >> > > First thanks for your good suggestion! > > All we realized that the raw data of IMX pad config in dts is not good. > e.g. > pinctrl_ecspi3: ecspi3grp { > fsl,pins = < > MX7D_PAD_SAI2_TX_SYNC__ECSPI3_MISO 0x17059 > ......... > > I did a deep feasibility analysis these days on the migrating to generic > pinctrl binding you referred. It is regrettably that it may not be so suitable > for IMX to fully migrate right now. > > Generic binding only supports parsing strings for pins and function > from dts right now, that means we need back to the old way of hardcoding > all register bits in driver which we actually chose to move out since > the commit "e164153 pinctrl: imx: move hard-coding data into device tree". > > And IMX doesn't have PAD GROUP in HW, each pad theoretically can be muxed > as 8 single functions. > e.g. > For IOMUXC_SW_MUX_CTL_PAD_CSI_DATA07 > MUX_MODE MUX Mode Select Field. > Select 1 of 8 iomux modes to be used for pad: CSI_DATA07. > 000 ALT0 — Select signal CSI1_DATA09. > 001 ALT1 — Select signal ESAI_TX3_RX2. > 010 ALT2 — Select signal I2C4_SDA. > 011 ALT3 — Select signal KPP_ROW7. > 100 ALT4 — Select signal UART6_CTS_B. > 101 ALT5 — Select signal GPIO1_IO21. > 110 ALT6 — Select signal EIM_DATA16. > 111 ALT7 — Select signal DCIC1_OUT. > By converting to generic binding, we need construct a huge function/group map > in driver for hundreds of pads for each SoC. That is a bit fear to us. > > Current IMX method only generates the used function/groups dynamically > by parsing board dts which is much a lighter way. > > But of course, we want to fix the raw config value issue as well which is > un-maintainable and un-readable as you pointed out. > > I think there might be two options for us to go: > One option is using macro definitions instead of raw data as pinctrl-single > users as OMAP. > e.g. > art2_pins: pinmux_uart2_pins { > pinctrl-single,pins = < > OMAP4_IOPAD(0x118, PIN_INPUT_PULLUP | MUX_MODE0) > /* uart2_cts.uart2_cts */ > ........ > > For this way, no driver changes required, just replace the raw data by macro > in device tree. But i wonder this may not be your original intention although > it's the easiest way for us. > Anyway, if you agree, we probably would prefer this way. > > Another option is partially convert to generic binding for only pad > configuration > part. e.g. > pinctrl_usdhc1: usdhc1grp { > fsl,pins = < > MX7D_PAD_SD1_CMD__SD1_CMD > MX7D_PAD_SD1_CLK__SD1_CLK > MX7D_PAD_SD1_DATA0__SD1_DATA0 > ... > >; > bias-pull-up = <47KOHM>; > drive-strength = <130OHM>; > slew-rate = <FAST>; > speed = <50MHZ>; > .... > }; > (Some of the property still not supported or not the same as generic > binding right now) > > This way requires no big drivers changes and only extend the driver > to support the generic pinctrl dt configuration properties. > > And it also fix the raw data issues and looks like more applicable than the full > migration. > > So would you be willing to accept it? > > Another potential benefits for this way is that we may can use PCONFDUMP > to dump a more readable data from pinctrl debug system. > >> Yours, >> Linus Walleij >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-clk" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > Regards > Dong Aisheng
On Thu, Feb 16, 2017 at 8:51 AM, Dong Aisheng <dongas86@gmail.com> wrote: > On Tue, Dec 27, 2016 at 8:59 PM, Linus Walleij <linus.walleij@linaro.org> wrote: >> I would consider augmenting the code in the driver to handle the generic >> bindings *in addition* to the old legacy bindings, and use those over these >> random custom bits. (...) > Generic binding only supports parsing strings for pins and function > from dts right now, that means we need back to the old way of hardcoding > all register bits in driver which we actually chose to move out since > the commit "e164153 pinctrl: imx: move hard-coding data into device tree". Aha I see. > One option is using macro definitions instead of raw data as pinctrl-single > users as OMAP. > e.g. > art2_pins: pinmux_uart2_pins { > pinctrl-single,pins = < > OMAP4_IOPAD(0x118, PIN_INPUT_PULLUP | MUX_MODE0) > /* uart2_cts.uart2_cts */ > ........ > > For this way, no driver changes required, just replace the raw data by macro > in device tree. But i wonder this may not be your original intention although > it's the easiest way for us. > Anyway, if you agree, we probably would prefer this way. That make it look better than it looks today, but... > Another option is partially convert to generic binding for only pad > configuration > part. e.g. > pinctrl_usdhc1: usdhc1grp { > fsl,pins = < > MX7D_PAD_SD1_CMD__SD1_CMD > MX7D_PAD_SD1_CLK__SD1_CLK > MX7D_PAD_SD1_DATA0__SD1_DATA0 > ... > >; > bias-pull-up = <47KOHM>; > drive-strength = <130OHM>; > slew-rate = <FAST>; > speed = <50MHZ>; > .... > }; > (Some of the property still not supported or not the same as generic > binding right now) This looks totally awesome, so if you could do this, it would be even better. > This way requires no big drivers changes and only extend the driver > to support the generic pinctrl dt configuration properties. Yep, you should be able to support both the old and the new way of configuring the pins this way I guess. > And it also fix the raw data issues and looks like more applicable than the full > migration. > > So would you be willing to accept it? Anything that move us closer to the generic bindings will be accepted. Yours, Linus Walleij
Hi Linus, On Tue, Mar 14, 2017 at 9:54 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Feb 16, 2017 at 8:51 AM, Dong Aisheng <dongas86@gmail.com> wrote: >> On Tue, Dec 27, 2016 at 8:59 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > >>> I would consider augmenting the code in the driver to handle the generic >>> bindings *in addition* to the old legacy bindings, and use those over these >>> random custom bits. > (...) >> Generic binding only supports parsing strings for pins and function >> from dts right now, that means we need back to the old way of hardcoding >> all register bits in driver which we actually chose to move out since >> the commit "e164153 pinctrl: imx: move hard-coding data into device tree". > > Aha I see. > >> One option is using macro definitions instead of raw data as pinctrl-single >> users as OMAP. >> e.g. >> art2_pins: pinmux_uart2_pins { >> pinctrl-single,pins = < >> OMAP4_IOPAD(0x118, PIN_INPUT_PULLUP | MUX_MODE0) >> /* uart2_cts.uart2_cts */ >> ........ >> >> For this way, no driver changes required, just replace the raw data by macro >> in device tree. But i wonder this may not be your original intention although >> it's the easiest way for us. >> Anyway, if you agree, we probably would prefer this way. > > That make it look better than it looks today, but... > >> Another option is partially convert to generic binding for only pad >> configuration >> part. e.g. >> pinctrl_usdhc1: usdhc1grp { >> fsl,pins = < >> MX7D_PAD_SD1_CMD__SD1_CMD >> MX7D_PAD_SD1_CLK__SD1_CLK >> MX7D_PAD_SD1_DATA0__SD1_DATA0 >> ... >> >; >> bias-pull-up = <47KOHM>; >> drive-strength = <130OHM>; >> slew-rate = <FAST>; >> speed = <50MHZ>; >> .... >> }; >> (Some of the property still not supported or not the same as generic >> binding right now) > > This looks totally awesome, so if you could do this, it would be even > better. > >> This way requires no big drivers changes and only extend the driver >> to support the generic pinctrl dt configuration properties. > > Yep, you should be able to support both the old and the new > way of configuring the pins this way I guess. > >> And it also fix the raw data issues and looks like more applicable than the full >> migration. >> >> So would you be willing to accept it? > > Anything that move us closer to the generic bindings will be accepted. > It's good to get an alignment with you. We will start to do that. Thanks Regards Dong Aisheng
diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt new file mode 100644 index 0000000..9561c1a --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt @@ -0,0 +1,37 @@ +* Freescale i.MX6 SLL IOMUX Controller + +Please refer to fsl,imx-pinctrl.txt in this directory for common binding part +and usage. + +Required properties: +- compatible: "fsl,imx6sll-iomuxc" +- fsl,pins: each entry consists of 6 integers and represents the mux and config + setting for one pin. The first 5 integers <mux_reg conf_reg input_reg mux_val + input_val> are specified using a PIN_FUNC_ID macro, which can be found in + imx6sll-pinfunc.h under device tree source folder. The last integer CONFIG is + the pad setting value like pull-up on this pin. Please refer to i.MX6SLL + Reference Manual for detailed CONFIG settings. + +CONFIG bits definition: +PAD_CTL_LVE (1 << 22) +PAD_CTL_HYS (1 << 16) +PAD_CTL_PUS_100K_DOWN (0 << 14) +PAD_CTL_PUS_47K_UP (1 << 14) +PAD_CTL_PUS_100K_UP (2 << 14) +PAD_CTL_PUS_22K_UP (3 << 14) +PAD_CTL_PUE (1 << 13) +PAD_CTL_PKE (1 << 12) +PAD_CTL_ODE (1 << 11) +PAD_CTL_SPEED_LOW (0 << 6) +PAD_CTL_SPEED_MED (1 << 6) +PAD_CTL_SPEED_HIGH (3 << 6) +PAD_CTL_DSE_DISABLE (0 << 3) +PAD_CTL_DSE_260ohm (1 << 3) +PAD_CTL_DSE_130ohm (2 << 3) +PAD_CTL_DSE_87ohm (3 << 3) +PAD_CTL_DSE_65ohm (4 << 3) +PAD_CTL_DSE_52ohm (5 << 3) +PAD_CTL_DSE_43ohm (6 << 3) +PAD_CTL_DSE_37ohm (7 << 3) +PAD_CTL_SRE_FAST (1 << 0) +PAD_CTL_SRE_SLOW (0 << 0)
Add pinctrl binding doc update for imx6sll. Signed-off-by: Bai Ping <ping.bai@nxp.com> --- .../bindings/pinctrl/fsl,imx6sll-pinctrl.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt