Hi Varadarajan,

On 3/24/25 09:05, Varadarajan Narayanan wrote:
Add pinctrl and GPIO driver for SA8775P. Driver code is based on the
similar U-Boot and Linux drivers.

Thanks, this mostly looks good.

[...]

+static const unsigned int sa8775p_pin_offsets[] = {
+         [0] = SA8775_PIN_OFFSET,   [1] = SA8775_PIN_OFFSET,   [2] = 
SA8775_PIN_OFFSET,
+         [3] = SA8775_PIN_OFFSET,   [4] = SA8775_PIN_OFFSET,   [5] = 
SA8775_PIN_OFFSET,
+         [6] = SA8775_PIN_OFFSET,   [7] = SA8775_PIN_OFFSET,   [8] = 
SA8775_PIN_OFFSET,
+         [9] = SA8775_PIN_OFFSET,  [10] = SA8775_PIN_OFFSET,  [11] = 
SA8775_PIN_OFFSET,

I guess we're really hitting the limit with the current model, and should probably just aligned the API here with the Linux one (adding struct msm_pingroup, etc).

That's a fair bit of work which isn't fair to drop in your lap, and I don't really see a better way of solving this with the current API (though I do question if the devicetree is just wrong here?), so I'll take this as-is.

That all being said, I feel like some justification for this would have been good, either in the commit message (or below it with --- separator), certainly it would have saved me the time of reverse-engineering why it was done this way.

Reviewed-by: Caleb Connolly <[email protected]>

Kind regards,
+        [12] = SA8775_PIN_OFFSET,  [13] = SA8775_PIN_OFFSET,  [14] = 
SA8775_PIN_OFFSET,
+        [15] = SA8775_PIN_OFFSET,  [16] = SA8775_PIN_OFFSET,  [17] = 
SA8775_PIN_OFFSET,
+        [18] = SA8775_PIN_OFFSET,  [19] = SA8775_PIN_OFFSET,  [20] = 
SA8775_PIN_OFFSET,
+        [21] = SA8775_PIN_OFFSET,  [22] = SA8775_PIN_OFFSET,  [23] = 
SA8775_PIN_OFFSET,
+        [24] = SA8775_PIN_OFFSET,  [25] = SA8775_PIN_OFFSET,  [26] = 
SA8775_PIN_OFFSET,
+        [27] = SA8775_PIN_OFFSET,  [28] = SA8775_PIN_OFFSET,  [29] = 
SA8775_PIN_OFFSET,
+        [30] = SA8775_PIN_OFFSET,  [31] = SA8775_PIN_OFFSET,  [32] = 
SA8775_PIN_OFFSET,
+        [33] = SA8775_PIN_OFFSET,  [34] = SA8775_PIN_OFFSET,  [35] = 
SA8775_PIN_OFFSET,
+        [36] = SA8775_PIN_OFFSET,  [37] = SA8775_PIN_OFFSET,  [38] = 
SA8775_PIN_OFFSET,
+        [39] = SA8775_PIN_OFFSET,  [40] = SA8775_PIN_OFFSET,  [41] = 
SA8775_PIN_OFFSET,
+        [42] = SA8775_PIN_OFFSET,  [43] = SA8775_PIN_OFFSET,  [44] = 
SA8775_PIN_OFFSET,
+        [45] = SA8775_PIN_OFFSET,  [46] = SA8775_PIN_OFFSET,  [47] = 
SA8775_PIN_OFFSET,
+        [48] = SA8775_PIN_OFFSET,  [49] = SA8775_PIN_OFFSET,  [50] = 
SA8775_PIN_OFFSET,
+        [51] = SA8775_PIN_OFFSET,  [52] = SA8775_PIN_OFFSET,  [53] = 
SA8775_PIN_OFFSET,
+        [54] = SA8775_PIN_OFFSET,  [55] = SA8775_PIN_OFFSET,  [56] = 
SA8775_PIN_OFFSET,
+        [57] = SA8775_PIN_OFFSET,  [58] = SA8775_PIN_OFFSET,  [59] = 
SA8775_PIN_OFFSET,
+        [60] = SA8775_PIN_OFFSET,  [61] = SA8775_PIN_OFFSET,  [62] = 
SA8775_PIN_OFFSET,
+        [63] = SA8775_PIN_OFFSET,  [64] = SA8775_PIN_OFFSET,  [65] = 
SA8775_PIN_OFFSET,
+        [66] = SA8775_PIN_OFFSET,  [67] = SA8775_PIN_OFFSET,  [68] = 
SA8775_PIN_OFFSET,
+        [69] = SA8775_PIN_OFFSET,  [70] = SA8775_PIN_OFFSET,  [71] = 
SA8775_PIN_OFFSET,
+        [72] = SA8775_PIN_OFFSET,  [73] = SA8775_PIN_OFFSET,  [74] = 
SA8775_PIN_OFFSET,
+        [75] = SA8775_PIN_OFFSET,  [76] = SA8775_PIN_OFFSET,  [77] = 
SA8775_PIN_OFFSET,
+        [78] = SA8775_PIN_OFFSET,  [79] = SA8775_PIN_OFFSET,  [80] = 
SA8775_PIN_OFFSET,
+        [81] = SA8775_PIN_OFFSET,  [82] = SA8775_PIN_OFFSET,  [83] = 
SA8775_PIN_OFFSET,
+        [84] = SA8775_PIN_OFFSET,  [85] = SA8775_PIN_OFFSET,  [86] = 
SA8775_PIN_OFFSET,
+        [87] = SA8775_PIN_OFFSET,  [88] = SA8775_PIN_OFFSET,  [89] = 
SA8775_PIN_OFFSET,
+        [90] = SA8775_PIN_OFFSET,  [91] = SA8775_PIN_OFFSET,  [92] = 
SA8775_PIN_OFFSET,
+        [93] = SA8775_PIN_OFFSET,  [94] = SA8775_PIN_OFFSET,  [95] = 
SA8775_PIN_OFFSET,
+        [96] = SA8775_PIN_OFFSET,  [97] = SA8775_PIN_OFFSET,  [98] = 
SA8775_PIN_OFFSET,
+        [99] = SA8775_PIN_OFFSET, [100] = SA8775_PIN_OFFSET, [101] = 
SA8775_PIN_OFFSET,
+       [102] = SA8775_PIN_OFFSET, [103] = SA8775_PIN_OFFSET, [104] = 
SA8775_PIN_OFFSET,
+       [105] = SA8775_PIN_OFFSET, [106] = SA8775_PIN_OFFSET, [107] = 
SA8775_PIN_OFFSET,
+       [108] = SA8775_PIN_OFFSET, [109] = SA8775_PIN_OFFSET, [110] = 
SA8775_PIN_OFFSET,
+       [111] = SA8775_PIN_OFFSET, [112] = SA8775_PIN_OFFSET, [113] = 
SA8775_PIN_OFFSET,
+       [114] = SA8775_PIN_OFFSET, [115] = SA8775_PIN_OFFSET, [116] = 
SA8775_PIN_OFFSET,
+       [117] = SA8775_PIN_OFFSET, [118] = SA8775_PIN_OFFSET, [119] = 
SA8775_PIN_OFFSET,
+       [120] = SA8775_PIN_OFFSET, [121] = SA8775_PIN_OFFSET, [122] = 
SA8775_PIN_OFFSET,
+       [123] = SA8775_PIN_OFFSET, [124] = SA8775_PIN_OFFSET, [125] = 
SA8775_PIN_OFFSET,
+       [126] = SA8775_PIN_OFFSET, [127] = SA8775_PIN_OFFSET, [128] = 
SA8775_PIN_OFFSET,
+       [129] = SA8775_PIN_OFFSET, [130] = SA8775_PIN_OFFSET, [131] = 
SA8775_PIN_OFFSET,
+       [132] = SA8775_PIN_OFFSET, [133] = SA8775_PIN_OFFSET, [134] = 
SA8775_PIN_OFFSET,
+       [135] = SA8775_PIN_OFFSET, [136] = SA8775_PIN_OFFSET, [137] = 
SA8775_PIN_OFFSET,
+       [138] = SA8775_PIN_OFFSET, [139] = SA8775_PIN_OFFSET, [140] = 
SA8775_PIN_OFFSET,
+       [141] = SA8775_PIN_OFFSET, [142] = SA8775_PIN_OFFSET, [143] = 
SA8775_PIN_OFFSET,
+       [144] = SA8775_PIN_OFFSET, [145] = SA8775_PIN_OFFSET, [146] = 
SA8775_PIN_OFFSET,
+       [147] = SA8775_PIN_OFFSET, [148] = SA8775_PIN_OFFSET, [148] = 
SA8775_PIN_OFFSET,
+       [149] = SA8775_PIN_OFFSET, [150] = SA8775_PIN_OFFSET, [151] = 
SA8775_PIN_OFFSET,
+       [152] = SA8775_PIN_OFFSET, [153] = SA8775_PIN_OFFSET, [154] = 
SA8775_PIN_OFFSET,
+};
+
+static const struct msm_pinctrl_data sa8775p_data = {
+       .pin_data = {
+               .pin_count = 155,
+               .special_pins_start = 149,
+               .special_pins_data = msm_special_pins_data,
+               .pin_offsets = sa8775p_pin_offsets,
+       },
+       .functions_count = ARRAY_SIZE(msm_pinctrl_functions),
+       .get_function_name = sa8775p_get_function_name,
+       .get_function_mux = sa8775p_get_function_mux,
+       .get_pin_name = sa8775p_get_pin_name,
+};
+
+static const struct udevice_id msm_pinctrl_ids[] = {
+       { .compatible = "qcom,sa8775p-tlmm", .data = (ulong)&sa8775p_data },
+       { /* Sentinal */ }
+};
+
+U_BOOT_DRIVER(pinctrl_sa8775p) = {
+       .name           = "pinctrl_sa8775p",
+       .id             = UCLASS_NOP,
+       .of_match       = msm_pinctrl_ids,
+       .ops            = &msm_pinctrl_ops,
+       .bind           = msm_pinctrl_bind,
+       .flags          = DM_FLAG_PRE_RELOC,
+};

base-commit: 244e61fbb7f5045e4e187024f7ae80434c952145

--
Caleb (they/them)

Reply via email to