On Fri, 2014-07-25 at 18:15 +0300, Ivan T. Ivanov wrote: > On Thu, 2014-07-24 at 17:23 -0700, Stephen Boyd wrote: > > On 07/24/14 08:40, Linus Walleij wrote: > > > On Thu, Jul 24, 2014 at 1:47 AM, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> wrote: > > > > > >>> Please add these constants to the table of valid power-source values and use > > >>> something like I did to translate them to register values - it makes the DT > > >>> much more readable. > > >> The DT could be similarly readable if we had a bunch of #defines for the > > >> different VIN settings that resolved to the final register value for > > >> that pmic. Something like PM8921_GPIO1_14_VPH, PM8921_GPIO19_36_VPH, > > >> etc. There would be a lot of them, but then the driver could be really > > >> simple and just jam whatever value is in the DT into the register > > >> without having to bounce through a mapping table in software to figure > > >> out the register value. This is ok. > If we did this for the functions also then I > > >> believe we achieve readability without requiring a bunch of drivers for > > >> each and every single pmic? > > > Not sure but it sounds like you want to make the device tree a jam table, > > > (know about individual register offsets, sequences etc). That has been > > > throrougly NACKed in the past, because DT is not Open Firmware. > > > > > > The exception is pinctrl-single which is restricted to single register > > > per pin use cases and is still a point of contention... > > > > > > > I'm not proposing a jam table. I'm proposing that we make the > > function/source property convenient to the driver by having the actual > > function field value encoded there instead of some string that has to be > > translated through a table in a driver. There's still going to be > > shifting and masking of bits in the driver to put the value in the right > > place in the register, but we avoid needing N number of drivers for each > > pmic just to translate strings into integers (for functions) and > > integers into other integers (for the power source). > > This sounds good to me. Drawback is that we will have custom > function parsing code, but I will try and see what that looks like. Hm, I played with this, using numbers for functions. Unfortunately it is not easy possible to hack existing DT pin-control parsing code to use numbers instead strings, so I ended coping Samsung parsing code, but unless this is separated in library we will end with huge code duplication for little benefit. Furthermore meaning of number 2 and 3, which represent PMIC GPIO Special Function 1 and 2 are not consistent across PMIC chips. For example KEYPD function in PM8038 is encoded with 3, but in PM8058 it is 2. I tend to agree with Bjorn that "function" property should be "normal", "paired", "func1", "func2","dtest1", "dtest2", "dtest3", "dtest4" and we can add new property "qcom,mode" which will select between digital/analog input/output. In DT include file we can still have something like this: /* To be used with "function = <>" */ #define QCOM_GPIO_FUNC_NORMAL "normal" #define QCOM_GPIO_FUNC_PAIRED "paired" #define QCOM_GPIO_FUNC_FUNC1 "func1" #define QCOM_GPIO_FUNC_FUNC2 "func2" ... #define PM8038_GPIO1_2_LPG_DRV QCOM_GPIO_FUNC_FUNC1 #define PM8038_GPIO3_5V_BOOST_EN QCOM_GPIO_FUNC_FUNC1 #define PM8038_GPIO4_SSBI_ALT_CLK QCOM_GPIO_FUNC_FUNC1 #define PM8038_GPIO5_6_EXT_REG_EN QCOM_GPIO_FUNC_FUNC1 #define PM8038_GPIO10_11_EXT_REG_EN QCOM_GPIO_FUNC_FUNC1 #define PM8038_GPIO6_7_CLK QCOM_GPIO_FUNC_FUNC1 #define PM8038_GPIO9_BAT_ALRM_OUT QCOM_GPIO_FUNC_FUNC1 #define PM8038_GPIO6_12_KYPD_DRV QCOM_GPIO_FUNC_FUNC2 #define PM8058_GPIO7_8_MP3_CLK QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO7_8_BCLK_19P2MHZ QCOM_GPIO_FUNC_FUNC2 #define PM8058_GPIO9_26_KYPD_DRV QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO21_23_UART_TX QCOM_GPIO_FUNC_FUNC2 #define PM8058_GPIO24_26_LPG_DRV QCOM_GPIO_FUNC_FUNC2 #define PM8058_GPIO33_BCLK_19P2MHZ QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO34_35_MP3_CLK QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO36_BCLK_19P2MHZ QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO37_UPL_OUT QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO37_UART_M_RX QCOM_GPIO_FUNC_FUNC2 #define PM8058_GPIO38_XO_SLEEP_CLK QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO38_39_CLK_32KHZ QCOM_GPIO_FUNC_FUNC2 #define PM8058_GPIO39_MP3_CLK QCOM_GPIO_FUNC_FUNC1 #define PM8058_GPIO40_EXT_BB_EN QCOM_GPIO_FUNC_FUNC1 #define PM8917_GPIO9_18_KEYP_DRV QCOM_GPIO_FUNC_FUNC1 #define PM8917_GPIO20_BAT_ALRM_OUT QCOM_GPIO_FUNC_FUNC1 #define PM8917_GPIO21_23_UART_TX QCOM_GPIO_FUNC_FUNC2 #define PM8917_GPIO25_26_EXT_REG_EN QCOM_GPIO_FUNC_FUNC1 #define PM8917_GPIO37_38_XO_SLEEP_CLK QCOM_GPIO_FUNC_FUNC1 #define PM8917_GPIO37_38_MP3_CLK QCOM_GPIO_FUNC_FUNC2 ... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
